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

Update Tempo's open-telemetry/collector dependency #1102

Closed
yvrhdn opened this issue Nov 5, 2021 · 4 comments · Fixed by #1383
Closed

Update Tempo's open-telemetry/collector dependency #1102

yvrhdn opened this issue Nov 5, 2021 · 4 comments · Fixed by #1383
Milestone

Comments

@yvrhdn
Copy link
Member

yvrhdn commented Nov 5, 2021

Is your feature request related to a problem? Please describe.

Tempo depends on go.opentelemetry.io/collector for its receivers. We are currently still using v0.21 while the latest version is already v0.38...

tempo/go.mod

Line 57 in c5d007d

go.opentelemetry.io/collector v0.21.0

The receivers are used by the distributor: https://github.com/grafana/tempo/search?q=go.opentelemetry.io%2Fcollector

Describe the solution you'd like

Update the dependency.
Update the documentation if this changes any defaults (I'm thinking about #637).

Describe alternatives you've considered

If we don't update we are going to miss out on security and performance improvements.

Additional context
N/A

@yvrhdn yvrhdn added this to the v1.3 milestone Nov 5, 2021
@yvrhdn yvrhdn changed the title Update open-telemetry/collector depedency Update Tempo's open-telemetry/collector dependency Nov 5, 2021
@yvrhdn
Copy link
Member Author

yvrhdn commented Nov 5, 2021

We should probably also look into updating our opentelemetry-proto submodule:

tempo/.gitmodules

Lines 1 to 3 in c5d007d

[submodule "opentelemetry-proto"]
path = opentelemetry-proto
url = https://github.com/open-telemetry/opentelemetry-proto

I don't expect any changes in the traces format, but it's good to stay fairly up-to-date.

Diff: open-telemetry/opentelemetry-proto@a17f202...main

@trexx
Copy link

trexx commented Dec 6, 2021

I take it that upgrading the collector dependency will also allow enabling TLS on the otlp receiver?
When I attempt to add the necessary TLS configuration in 1.2, Tempo distributor complains about invalid configuration.

@annanay25
Copy link
Contributor

annanay25 commented Jan 12, 2022

The collector was updated in the following PRs

Should we rename this issue (to only update the proto) and move it into the next milestone?

@yvrhdn
Copy link
Member Author

yvrhdn commented Jan 12, 2022

I'm not sure if we want to bother updating the proto. OTLP traces is declared stable so not much has changed.
These are changes between our version and main: open-telemetry/opentelemetry-proto@a17f202...main

Maybe we can look into this when we work on new backend formats?

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 a pull request may close this issue.

4 participants