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

Add OTEL_SERVICE_VERSION environment variable. #3573

Closed

Conversation

carlosalberto
Copy link
Contributor

As service.version was recently marked as stable, we should (IMHO) add an environment variable for it (there's a moratorium on environment variables but this should be allowed, as it complements the existing OTEL_SERVICE_NAME variable).

Btw, we still reference our copy of the semantic conventions, and we should update such links.

@carlosalberto carlosalberto requested review from a team June 28, 2023 14:30
@reyang
Copy link
Member

reyang commented Jun 28, 2023

@carlosalberto would you update the changelog?

@Oberon00
Copy link
Member

I feel like this is not nearly as important as OTEL_SERVICE_NAME. If the generic resource environment variable is too cumbersome to use, maybe we should address that instead of introducing more special cases.

@github-actions
Copy link

github-actions bot commented Jul 6, 2023

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

@github-actions github-actions bot added the Stale label Jul 6, 2023
@carlosalberto
Copy link
Contributor Author

I'd argue it has some value as it brings completeness to the Resource's service section. IMHO it's not a big change neither. Let's discuss the pros and cons in the Spec call next Tuesday.

@github-actions github-actions bot removed the Stale label Jul 7, 2023
@github-actions
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 the Stale label Jul 19, 2023
@tylerbenson
Copy link
Member

Adding my 2 cents...
By promoting version to an independent environment variable configuration we signal to the users that (like service name) this is important and something they should populate.

With the complexities of modern deployments, I think it's very helpful for users to understand which version of their app generated the signals they are observing and be able to distinguish data in a partial deployment.

Telling users to put it in the resource map suggests that it's more optional and probably not useful.

Can we please move forward with this proposal?

yurishkuro
yurishkuro previously approved these changes Jul 19, 2023
@yurishkuro yurishkuro dismissed their stale review July 19, 2023 15:31

change my mind

@yurishkuro
Copy link
Member

Agree with @Oberon00 - redundancy is not good, and making this part of the spec creates a bunch of work in the SDK SIGs for a capability that is already available, only for the sake of making it "more visible". Instead we can make improvements in the documentation, recommending people to set a version.

@github-actions github-actions bot removed the Stale label Jul 20, 2023
@carlosalberto
Copy link
Contributor Author

@yurishkuro I agree with it in general, but we already do this massaging for OTEL_SERVICE_NAME, so we are merely adding a value. Now, should we add more values that do this same massaging? Most likely not. But for completeness I'd vow to add this value for now (once the config proposal reaches stability, we can forget about this).

Maybe @pellared @MikeGoldsmith want to chime in? I remember interest in this specific value.

@pellared
Copy link
Member

I think that OTEL_SERVICE_VERSION has less value than OTEL_SERVICE_NAME as service.name is a Required resource attribute. We often require users to set the service.name while the same cannot be said for service.version.

@pellared
Copy link
Member

I would rather consider something more generic like #3025. However, I do not see a not of interest in both issues. Therefore, I think the best think is to not change anything at the moment.

@github-actions
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 the Stale label Jul 29, 2023
@github-actions
Copy link

github-actions bot commented Aug 6, 2023

Closed as inactive. Feel free to reopen if this PR is still being worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants