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

resource: Allow service.version to be auto-populated by SDKs #1338

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Aug 16, 2024

Changes

  • Allow SDKs to offer an opt-in feature to auto-populate service.version if there are standard ways to do it for the SIG

Details

.NET has some built-in mechanisms for versioning assemblies: Assembly versioning.

What this PR seeks to do is make it allowable for SDKs to populate service.version from known sources if the user explicitly enables/requests auto-population.

Here is a PoC branch I did of what this might look like in .NET SDK: https://github.com/open-telemetry/opentelemetry-dotnet/compare/main...CodeBlanch:resource-service-version-auto-resolution?expand=1

This support was requested by a user: open-telemetry/opentelemetry-dotnet#5703.

Merge requirement checklist

@CodeBlanch CodeBlanch requested review from a team August 16, 2024 21:34
Copy link
Contributor

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@trask
Copy link
Member

trask commented Aug 22, 2024

since this is SDK spec, should it be in opentelemetry-specification instead?

@joaopgrassi
Copy link
Member

joaopgrassi commented Aug 23, 2024

since this is SDK spec, should it be in opentelemetry-specification instead?

Yeah I was thinking the same when I was reviewing. It feels similar to telemetry sdk attributes https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/README.md#telemetry-sdk.

@lmolkova
Copy link
Contributor

since this is SDK spec, should it be in opentelemetry-specification instead?

Yeah I was thinking the same when I was reviewing. It feels similar to telemetry sdk attributes https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/README.md#telemetry-sdk.

FWIW spec doesn't mention service.version much https://github.com/search?q=repo%3Aopen-telemetry%2Fopentelemetry-specification+service.version&type=code

And delegates the definition of service.name to semconv.

I feel we need to update semconv anyway.

Let's bring it up in Monday SemConv and/or Tue Spec call to highlight this change and discuss whether we should have something in the spec about service.*.

@@ -20,6 +20,13 @@ groups:
type: string
brief: >
The version string of the service API or implementation. The format is not defined by these conventions.
note: |
When possible, SDKs MAY use language/platform-specific capabilities to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added Stale and removed Stale labels Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

5 participants