Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change behaviour of ELASTIC_APM_INSTRUMENT config #467

Closed
alvarolobato opened this issue Jul 4, 2019 · 5 comments
Closed

Change behaviour of ELASTIC_APM_INSTRUMENT config #467

alvarolobato opened this issue Jul 4, 2019 · 5 comments
Assignees

Comments

@alvarolobato
Copy link
Contributor

alvarolobato commented Jul 4, 2019

Add ELASTIC_APM_INSTRUMENT for disabling instrumentation modules. This should not affect OpenTracing or other custom instrumentation.

See elastic/apm#49


Update @mikker: Currently blocked by elastic/apm#92 which will probably move this and a few other things around.

@mikker
Copy link
Contributor

mikker commented Nov 28, 2019

  1. We already have an option called instrument but it means something else. Right now it switches all transactions on/off (and thereby also spans). Keeping error tracking.

  2. Changing the behaviour of this would be a breaking change. If users want the functionality to match the new definition of this, they can disable most auto-instrumentation with config.disabled_instrumentations = config.available_instrumentations.

  3. If the option is to be available via CentralConfig we also need to change the spies. Disabling one right now means not loading its Spy, ie. file. Every spy would need to have a hot-swappable disabled flag like for example the net/http one has. That way we'll load all files, then disable/enable the ones the user has chosen.

  4. We also don't have a way to distinguish between manually started transactions and spans and agent-defined ones. I suppose we'd need to add some sort of flag to the start_X methods to distinguish, eg. ElasticAPM.start_span 'test', internal: true or auto: true or from_agent: true. All of them feel a bit weird but I do not want to put the responsibility of adding a flag on the user.

@estolfo, @axw wdyt?

@estolfo
Copy link
Contributor

estolfo commented Nov 28, 2019

How does this relate to elastic/apm#92 ? It is a completely separate issue?

I would also bring up these important points in the agent issue, as Thomas said something similar to (4) regarding the node agent.

@mikker
Copy link
Contributor

mikker commented Nov 28, 2019

This issue was created before that discussion but you're right that it's deeply affected by it. I think we should sit on this and maybe resolve it along with that other issue instead which is still (sigh) being debated. Agree?

@mikker mikker changed the title Add ELASTIC_APM_INSTRUMENT config Change behaviour of ELASTIC_APM_INSTRUMENT config Nov 28, 2019
@estolfo
Copy link
Contributor

estolfo commented Nov 28, 2019

Yes, this seems related to the other issue and they should be resolved/decided together.

@estolfo estolfo removed this from the 7.6 milestone Nov 29, 2019
@mikker
Copy link
Contributor

mikker commented May 26, 2020

Closing as instrument will be replaced by recording.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants