Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
RFC - Pipeline Component Telemetry #11406
base: main
Are you sure you want to change the base?
RFC - Pipeline Component Telemetry #11406
Changes from 2 commits
5df52e1
4232821
d0f1637
bea0e2f
35c82a4
2600836
b1fd90c
90d19ab
497587c
6463759
1b26ae2
bba26c9
7925012
3211be7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
OTel SemConv guidelines require metric attributes to be namespaces so they can be placed in a global registry along with resource attributes. I don't think we can use
otel
for collector. It probably should beotelcol
. Orotel.collector
instead. However,otel.
seems like kind of a restricted namespace. Not sure if we can use it or not. I think we need to run this by the otel semantic conventions SIG group before merging the RFC.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.
@dmitryax if you think contacting the semantic convention SIG is a requirement for this RFC to be merged, please request changes on the PR so that we follow the process
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.
I'm fine with changing the attribute names but delaying this functionality further for an entirely separate naming process seems unnecessary. Can we really not choose our own attributes names?
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 sentence from the doc linked by @dmitryax makes me hesitant about keeping the current approach:
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.
I agree we can't just use the
otel
namespace. Can't we choose another on our own though? I like theotelcol
suggestion.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.
otelcol
works for me, it's also the one we use for metrics so it makes sense to be consistent hereThere 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.
@mx-psi asked me to chime in to help interpret the spec.
You are reading it correctly. If you want to use
otel.
prefix it needs to go through semconv group.What can you use instead? If you read the preceding paragraph the closest applicable rules seems to be this:
If we consider "OpenTelemetry" the "company" then we should use "io.opentelemetry." as the org-specific prefix, e.g. "io.opentelemetry.collector." for the Collector.
However, I am not sure I like such a long prefix. "otelcol" or "otel.collector" looks much nicer to me.
I cannot find spec recommendations about when a new top-level namespace (like "otelcol") is allowed to be used. I would argue the spec should have a rule like this and the rule should be that it needs to go through semconv, just like for anything under "otel." namespace.
My recommendation is this:
You don't need to be blocked or wait until acceptance by semconv group, there is a ton of work that needs to happen anyway and replacing metric names should be trivial anytime before you declare them stable.
One additional thing that I think is worth thinking about: is this Collector-only telemetry or it can be applicable to SDK processors/exporters? Collector's telemetry is going to be much richer and needs to be more expressive, so I am not sure if it is beneficial to model Collector and SDK the same way. However, if we end up deciding that indeed we want to model them the same way, with SDK's use cases being virtually a subset of Collector's use cases, then "otel." prefix seems to be the most natural choice.
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.
Thanks @tigrannajaryan. I'll open an issue to add
otelcol
as a top level namespace in the semantic conventions.This is intended to be collector-only telemetry. It is attributed to the collector's instances of components, which are governed by a model that is not intended to be aligned with other parts of the project.
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.
I've opened open-telemetry/semantic-conventions#1555 to propose
otelcol
as a new top-level namespace.