Allow language SDKs to expand the scope of the service resource detector - #5280
Allow language SDKs to expand the scope of the service resource detector#5280mmanciop wants to merge 2 commits into
service resource detector#5280Conversation
Pull request dashboard statusWaiting on reviewers · refreshed 2026-09-01 14:32 UTC Review the latest changes. Status above doesn't look right?
|
| on [OTEL_SERVICE_NAME](../configuration/sdk-environment-variables.md#general-sdk-configuration) | ||
| environment variable; populates `service.instance.id` | ||
| * `service`: `service`: Populates `service.name` from the [OTEL_SERVICE_NAME](../configuration/sdk-environment-variables.md#general-sdk-configuration) | ||
| environment variable and MAY fall back to language- or platform-specific |
There was a problem hiding this comment.
this is under the header "#### Resource detector name", an in-development section. I think you meant a different section, since the intention is to change SDK's default behavior.
https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/service.md is the place where the current behavior is defined, so naturally, I think the change belongs there.
(This is one of the rare areas in the SDK Specification, which points to sem-convention.)
There was a problem hiding this comment.
That is also needed as a follow-up (and it is mentioned in the issue: #5276), but IMO first comes opening up the specification
There was a problem hiding this comment.
That is also needed as a follow-up
I'm not sure about that. Accepting this PR as written wouldn't open anything up; language SDKs would have nothing to change on the basis of it.
The spec defines how service.name is sourced from user configuration (OTEL_SERVICE_NAME , and its precedence over OTEL_RESOURCE_ATTRIBUTES ), but it defers what happens when the user hasn't provided a value to semantic conventions which says "If the value was not specified, SDKs MUST fallback to unknown_service: concatenated with the process executable name."
That's the requirement we need to change to achieve the desired goal. I don't think this unblocks implementations on its own.
There was a problem hiding this comment.
But that is exactly the thing: the behavior I am proposing is not something that I don't think semantic conventions should specify. These behaviors are SDK-, language-specific implementations.
I read the current spec as: doing anything different than OTEL_SERVICE_NAME and unknown_service:... is illegal.
There was a problem hiding this comment.
I read the current spec as: doing anything different than OTEL_SERVICE_NAME and unknown_service:... is illegal.
But where does it say so in the spec? "unknown_service" is not something in the spec at all. Its only defined in sem. conv.
Separately: The section this PR edits: its just about Resource detector names, and not about actual behaviors. Its odd that service alone has behavior being specified here, instead of pointing to the behavior specified elsewhere. We should not be introducing normative requirements to a Naming section.
There was a problem hiding this comment.
So this spec controls the behavior of the named service resource detector, which shows up in the declarative config schema at path $.resource.detection/development.detectors[*].service
Usage:
resource:
detection/development:
detectors:
- service:
- container:
- host:
- process:
tracer_provdier: ...
meter_provider: ...
logger_provider: ...
If we change the service detector definition to give the spec blessing to populate service.name for other language / framework specific sources, then an injector user can have a single config like the one above and get reasonable service.name values. Right now, the service detector only does anything to service.name if OTEL_SERVICE_NAME is set, which is impractical to set in the context of injector running on a linux box trying to instrument all processes for supported languages.
There was a problem hiding this comment.
If we change the service detector definition to give the spec blessing to populate service.name for other language / framework specific sources, then an injector user can have a single config like the one above and get reasonable service.name values.
mm. If we change the text here, (as in this PR), is that sufficient for all language sdks to modify their service.name detection to match the new behavior (if they chose to)? There are languages which don't support declarative config, and don't use named detectors - they won't change their behavior based on this PR. Is that what this issue is trying to tackle - i.e let sdk populate service name from additional source, if the sdk use declarative config and uses named detectors? My impression is that we are trying to fix the default for all sdks, irrespective of whether they use named detectors or decl. config.
There was a problem hiding this comment.
My understanding is that this PR is only trying to impact named service detectors as used in declarative config (and potentially elsewhere if the language uses these named detectors in a generic capacity). The base SDK's default resource behavior would go unchanged.
Correct me if I misunderstand @mmanciop
There was a problem hiding this comment.
The base SDK's default resource behavior would go unchanged.
Thanks. Yes I'll wait for @mmanciop to confirm if this is scoped to just named service detectors. If that is the case, I'll re-review with that angle in mind (Sorry I was under the impression that we are trying to change defaults for all) Given that is only going to touch in-developemnt parts, this is the perfect time to polish it to make it give better default to all.
There was a problem hiding this comment.
My understanding is that this PR is only trying to impact named service detectors as used in declarative config (and potentially elsewhere if the language uses these named detectors in a generic capacity). The base SDK's default resource behavior would go unchanged.
Correct me if I misunderstand @mmanciop
Update: I think I understand the confusion now @cijothomas, let me try to explain.
This is about the service detector, whether it is triggered by declarative config, or used in the initialization of SDKs. As I said in the maintainer call, as an end-user I put a lot of value in good defaults, and currently our default service.name is everything but. That is, I would allow SDKs to modify their service resource detector to perform opinionated, language-specific behaviors. That, by extension, works also in declarative configuration.
Expand the accepted scope of the built-in `service` resource detector to permit language- and framework-specific discovery of `service.name`. Closes open-telemetry#5276
0b00927 to
d41906b
Compare
| environment variable; populates `service.instance.id` | ||
| * `service`: Populates `service.name` from the [OTEL_SERVICE_NAME](../configuration/sdk-environment-variables.md#general-sdk-configuration) | ||
| environment variable and MAY fall back to language- or platform-specific | ||
| sources (for example `spring.application.name`, a JAR manifest, or a |
There was a problem hiding this comment.
Somehow we need to make it clear that its up to the maintainers discretion to decide to what degree they want to take on the burden of owning more complexity for the sake of supporting more framework specific patterns.
There was a problem hiding this comment.
Would MAY be better here?
There was a problem hiding this comment.
at the discretion of the specific SDK tries to convey this I think, although it could get an extra line at the end - something along the lines of Maintainers are responsible for deciding how much complexity the detector can incur into for the sake of better service name detection.
This comment has been minimized.
This comment has been minimized.
|
/dashboard route:reviewers |
|
@mmanciop, this pull request was routed to reviewers. |
|
@cijothomas is there anything blocking from your side? Other than minor things, we seem fine perhaps? |
6 participants
Fixes #5276
Changes
Open up the definition of the
serviceresource detector to allow for language-specific fallbacks when theOTEL_SERVICE_NAMEenvironment variable is not set.serviceresource detector #5276CHANGELOG.mdfile updated for non-trivial changes[chore]in the PR title to skip the changelog check