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

Clarify "automatic instrumentation" term #2700

Merged
merged 3 commits into from
Jul 28, 2022

Conversation

pellared
Copy link
Member

@pellared pellared commented Jul 27, 2022

Fixes #2336

Relates to open-telemetry/opentelemetry-dotnet-instrumentation#958

Relates to open-telemetry/opentelemetry.io#1565

Changes

Clarify "automatic instrumentation" term by removing some fragments that can bring confusion.

  1. Remove "to use the OpenTelemetry APIs" which some may think that library instrumentation is a type of automatic instrumentation
  2. Remove "access application code". Personally, I think that e.g. "assembly weaving" (instrumentation done as part of "compilation") is also a kind of automatic instrumentation. For some languages, like .NET, source code is not required, but I imagine some interpreted languages require accessing the code anyway 🤷 I just think that we can remove this part.
  3. Change "application code" to "application's source code". To make it more obvious that it is "source code" and e.g. not "bytecode".

@pellared pellared requested review from a team July 27, 2022 08:03
@pellared
Copy link
Member Author

@tedsuo @cartermp @chalin @cijothomas PTAL

@pellared
Copy link
Member Author

PTAL @open-telemetry/instr-wg @open-telemetry/java-instrumentation-approvers @open-telemetry/dotnet-instrumentation-approvers

@yurishkuro
Copy link
Member

I don't think we should be removing reference to OTEL APIs, on the contrary, we should be emphasizing it, but clarifying. Specifically, an auto-instrumentation that attaches to the code at runtime and somehow exports OTEL-compatible data, would NOT be a compliant auto-instrumentation in my view, because:

  • it breaks the API/SDK separation envisioned in the OTEL
  • it likely does not allow adding manual instrumentation on top of automatic, because the manual instrumentation has to go through OTEL API.

@pellared
Copy link
Member Author

pellared commented Jul 27, 2022

@yurishkuro How about adding:

The automatic instrumentation MUST:

- use (directly or indirectly) the OpenTelemetry APIs,
- allow adding manual instrumentation.

? Any better proposal?

However, I am not sure if this kind of detail should be contained in the glossary. Maybe there should be a dedicated document/spec describing automatic instrumentation (it rather should not be part of this PR).

@pellared pellared requested a review from cartermp July 28, 2022 06:04
@pellared
Copy link
Member Author

I will be AFK for a month. If there will be any blocking feedback then I am OK if someone addresses it on my branch. I allowed edits and access to secrets by maintainers. Of course, this PR can also be closed and someone can open a new one.

But maybe it is already good enough to be merged 🤞

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

Successfully merging this pull request may close these issues.

Clarify the definition of automatic instrumentation?
7 participants