apm-agent-python icon indicating copy to clipboard operation
apm-agent-python copied to clipboard

Allow flexibility in automatic instrumentation

Open ramshaw888 opened this issue 6 years ago • 1 comments

Is your feature request related to a problem? Please describe. Currently we have a single INSTRUMENTED config value that turns all instrumentation on/off. I think it would be useful to have some more flexibility.

In my particular use case for example, I would like to customise the requests instrumentation in my project which requires me to disable the out-of-the-box instrumentation. The easiest way I've found to do this is:

  • Set INSTRUMENTED: false in config
  • Vendor in logic found in elasticapm.instrumentation.register, elasticapm.instrumentation.control, to customising the _cls_register set to include my own instrumentation for requests.
  • (Additionally for flask) Subclass elasticapm.contrib.flask.ElasticAPM, overriding the init_app method so that I can register the request_started, request_finished signals.

Side note re: flask - I think this may be a bug, depending on your perspective. In other places in the project e.g. Django instrumentation, the INSTRUMENTED config value refers only to instrumentation logic found in elasticapm.instrumentation (AbstractInstrumentedModule subclasses). Whereas in the flask module, it additionally prevents registering of the above flask signals.

Describe the solution you'd like A way to customise exactly what gets instrumented from config, opt-in and opt-out.

Describe alternatives you've considered

Possible implementations:

  • Exclusion/inclusion override lists of modules to instrument, e.g.
Config:
    instrument_exclude = ["requests", "celery"]
    instrument_include = [("requests", my_custom_instrumentation_module_name)]

or

Config:
    instrument_overrides = [
        ("requests", my_custom_instrumentation_module_name),  # to replace
        ("celery", None),  # to disable
    ]
  • Config value that overrides elasticapm.instrumentation.register._cls_register. A "raw" solution. This however would require the client to specify the module names which are internal to elasticapm (probably less than ideal).

Does anyone have any thoughts on this? Please correct me if I have overlooked something that makes this easy in the current version. Thanks.

ramshaw888 avatar Apr 29 '19 04:04 ramshaw888

Hi @ramshaw888

I agree, this is a feature we'd really like to implement. Unfortunately, there is one roadblock that I wasn't able to get around yet: All our configuration is per-agent-instance and loaded when the agent is instantiated, while instrumentation is process wide and needs to be executed as early as possible during startup of the app. This makes it basically impossible to use our existing configuration setup for the list of instrumentations. I considered limiting this particular setting to environment variables only, but defining a long list of import paths via environment variable is pretty cumbersome.

beniwohli avatar May 03 '19 08:05 beniwohli