-
Notifications
You must be signed in to change notification settings - Fork 164
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
Add InstrumentationLibrary to describe telemetry from a named Tracer/Meter #84
Conversation
29107c3
to
100cc7a
Compare
This looks great to me, specially given we are not modifying the API itself. My opinion on two mentioned issues:
|
I agree.
The name and version of the component are the values passed to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this proposal, component
represents the telemetry library, not the component being instrumented correct?
For instance, if an autoinstrumentation library io.opentelemetry.contrib.auto-mysql
version semver:3.0.2
, which instruments a library mysql
version semver:5.1.2
, acquires named tracer with getTracer("io.opentelemetry.contrib.auto-mysql", "semver:3.0.2")
, the component is meant to be { name: io.opentelemetry.contrib.auto-mysql, version: semver:3.0.2 }
?
What should happen in the, admittedly contrived, case that a library instruments more than one thing? For instance, if an ORM which handles multiple database libraries acquires a tracer, spans for all database queries from the ORM would have the same component regardless of which database library is actually used to serve the request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggesting changes, because it is dangerous to mix up instrumenting and instrumented libraries.
The name used to create a Tracer or Meter must identify the instrumentation libraries (also referred to as integrations) and not the library being instrumented.
In open-telemetry/opentelemetry-specification#354, there's a question about how to talk about the values passed to the named tracer. "Instrumenting library"? |
Do you mean the values passed to the tracer provider? If so, then the OTEP (and incoming spec PR) is using the term "instrumentation library" (but it would not be incorrect to call it the instrumenting library). This is mainly to distinguish the instrumenting library from the instrumented library. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple things that are not quite clear to me.
The intent of the “component” concept appears to be “the call-site that emits the telemetry”. The telemetry may be emitted to describe a behaviour of a code that resides elsewhere and may even be written by someone else (e.g. a 3rd party library that is being instrumented). The “component” does not unambiguously map 1:1 to source-code entities such as a source-code file, a source-code directory, a library or a package. A “component” is merely a grouping of telemetry data by an arbitrary criteria as it is seen fit by the developer who decides to emit that telemetry data using the same (or same-named) NamedTracer/NamedMeter object.
This does not limit the “component” to a particular instrumenting library either since NamedTracer/NamedMeter object may be passed around freely and can be used to emit telemetry about virtually anything the developer deems fit.
It appears that we want to make recommendations around how NamedTraced/NamedMeter should be used when one creates an instrumenting code for some other code that is being instrumented. If so then these recommendations should probably be part of OpenTelemetry documentation (if they are not already).
If the above few paragraphs are true then the “component” as I understand it is merely a logical grouping of telemetry data. Using “components” for instrumenting libraries is probably a frequent use-case, but perhaps not the only case (e.g. one can imagine a case when different parts of the Application are considered different “components” even though they are not libraries at all and are parts of the Application codebase).
Given such definition of “component” I do not know whether it is warranted to make them a hard-coded part of the protocol as opposed to simply storing the component name in Span and Metric attributes. The earlier can be seen as an optimization of the later for the case when the same component can emit a large number of Spans or Metrics. Either approach seems reasonable to me and I have no data to have a strong opinion one way or another.
It would be useful to see clarification on these matters in this OTEP or elsewhere.
Honestly, I'm a bit confused how people are so confused about named tracers all the time. It seems people just hear the name and immediately in their mind they have an idea what that is/should be and then nobody reads the actual definitions what that "name" is. At this point maybe it's really better to rename the feature to ORI (Obligatory Reporter Info), then people have to read the docs. Or am I wrong and is this OTEP not misunderstanding but attempting to completely change the definition of the tracer name?
You can technically do that but the definition of the tracer name and what it is supposed to mean is very clear and explicit.
We already have definitions how named tracers MUST be used (maybe it's not 100% as clear for meters). |
@tigrannajaryan I am not sure I follow the entire comment. The rational to have this as a separate entity is for:
I am not sure why do we try to flatten everything when we do have this structure in the API, so it will be natural to use the same structure in the data model and not do any flattening. Also if we do use an attribute in the Span: for this exercise let's call it |
What can we do to move this forward? |
I am re-writing the otep using InstrumentationInfo as a replacement for the Component. Will have something in 1h |
@open-telemetry/specs-approvers please review this proposal again. |
…Meter Signed-off-by: Bogdan Cristian Drutu <bogdandrutu@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks better to me than the previous version.
Ping @open-telemetry/specs-approvers :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from my last comment in the Future possibilities, only copy editing, LGTM!
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
…Meter (open-telemetry#84) * Add InstrumentationLibrary to describe telemetry from a named Tracer/Meter Signed-off-by: Bogdan Cristian Drutu <bogdandrutu@gmail.com> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
…Meter (open-telemetry#84) * Add InstrumentationLibrary to describe telemetry from a named Tracer/Meter Signed-off-by: Bogdan Cristian Drutu <bogdandrutu@gmail.com> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
…Meter (open-telemetry#84) * Add InstrumentationLibrary to describe telemetry from a named Tracer/Meter Signed-off-by: Bogdan Cristian Drutu <bogdandrutu@gmail.com> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
…Meter (open-telemetry/oteps#84) * Add InstrumentationLibrary to describe telemetry from a named Tracer/Meter Signed-off-by: Bogdan Cristian Drutu <bogdandrutu@gmail.com> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> * Update text/0083-component.md Co-Authored-By: Christian Neumüller <christian+github@neumueller.me> Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
No description provided.