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 building of OTLP bindings #1

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

anuraaga
Copy link
Contributor

@anuraaga anuraaga commented Oct 5, 2021

No description provided.

@anuraaga anuraaga requested a review from jkwatson October 5, 2021 09:25
withType<Jar>().configureEach {
manifest {
attributes(
"Automatic-Module-Name" to "io.opentelemetry.proto",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important

publishing {
publications {
register<MavenPublication>("mavenPublication") {
groupId = "io.opentelemetry.proto"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkwatson The coordinates don't collide because this group is different.

Copy link

Choose a reason for hiding this comment

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

cool. yes, you are correct. thanks for re-iterating!

Copy link

Choose a reason for hiding this comment

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

so the maven coordinates end up as io.opentelemetry.proto:opentelemetry-proto, correct?

}

pom {
name.set("OpenTelemetry Protocol")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important

id("otel.publish-conventions")
}

description = "Java Bindings for the OpenTelemetry Protocol (OTLP)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important(ish)

@@ -0,0 +1,14 @@
version = 0.9.0-alpha
Copy link

Choose a reason for hiding this comment

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

the latest proto version is 0.10.0. Shouldn't we align with that initially, if this is going to be aligned with the proto releases?

Copy link
Contributor Author

@anuraaga anuraaga Oct 5, 2021

Choose a reason for hiding this comment

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

I figured we can release 0.9 first, remove proto from the SDK repo while then bumping this repo to 0.10. How does that sound?

Copy link

Choose a reason for hiding this comment

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

I think that it probably doesn't matter to wait for the SDK, unless we're using identical maven coordinates. I don't think we can use identical maven coordinates, since the versions will collide in the 0.10.0 release: https://search.maven.org/artifact/io.opentelemetry/opentelemetry-proto/0.10.0/jar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't wait for the SDK for anything here - I figured it's worth publishing both 0.9.0 and 0.10.0 here since it's easy, release, change one line, release once more. Having 0.9.0 would allow us to swap it in for the current proto module in the SDK without worry of any changes. Perhaps it's too much worry though given we only use it in tests there now.

Copy link

@jkwatson jkwatson Oct 5, 2021

Choose a reason for hiding this comment

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

ok, I'm fine with that. 👍🏽

@jkwatson
Copy link

jkwatson commented Oct 5, 2021

What are the resulting maven coordinates of this publication? I assume they don't collide with the one that we've been publishing in opentelemetry-java?

# This file controls who is tagged for review for any given pull request.

# For anything not explicitly taken by someone else:
* @anuraaga @bogdandrutu @jkwatson
Copy link

Choose a reason for hiding this comment

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

we should find some maintainers for this repo who care about having an independent proto artifact. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hoping this will be a pretty trivial repo after the initial commits :)

Copy link

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

:shipit:

@anuraaga anuraaga merged commit 2cec180 into open-telemetry:main Oct 6, 2021
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.

2 participants