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

Introduce a top-level Version() and SemVersion() function #225

Merged
merged 5 commits into from
Aug 16, 2020

Conversation

evantorrie
Copy link
Contributor

@evantorrie evantorrie commented Aug 15, 2020

These are intended to be used by submodules within the repo that need to specify an "instrumentation version" when creating a tracer or meter.

See #214

SemVersion() may not be required (opinions wanted), but it does enable creating a versioned tracer/meter simply while guaranteeing the correct semver: prefix is always applied.

tracer := otelglobal.TracerProvider().Tracer(
			defaultTracerName,
			oteltrace.WithInstrumentationVersion(contrib.SemVersion()),
		)

Bikeshedding on file name and location also welcomed.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 15, 2020

CLA Check
The committers are authorized under a signed CLA.

These are intended to be used by submodules within the repo that need
to specify an "instrumentation version" when creating a tracer or meter.
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

This looks good, and I think it should be merged as is (save the minor suggestions/questions).

Going forward we might want to consider keeping the version defined as a const that is updated at build time. I've seen this used before (with the related Makefile) and I think it would help when future bug reports come in. It would be easier to determine if the instrumentation was built from a particular commit or branch instead of a release.

I can open an issue if there any other shared desire to try this later on.

contrib.go Outdated Show resolved Hide resolved
pre_release.sh Outdated Show resolved Hide resolved
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@evantorrie
Copy link
Contributor Author

Going forward we might want to consider keeping the version defined as a const that is updated at build time. I've seen this used before (with the related Makefile) and I think it would help when future bug reports come in. It would be easier to determine if the instrumentation was built from a particular commit or branch instead of a release.

I can open an issue if there any other shared desire to try this later on.

Yes, I too like having the git shortref in the code, and I'm happy to take on the assignment.

@Aneurysm9
Copy link
Member

Yes, I too like having the git shortref in the code, and I'm happy to take on the assignment.

I created #229 to track that enhancement and noted an alternative approach that might work in place of a build-time declaration that would be out of our control.

@evantorrie evantorrie deleted the semversion-constant branch August 16, 2020 19:37
plantfansam referenced this pull request in plantfansam/opentelemetry-go-contrib Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants