Skip to content

chore: use jaeger-idl/proto-gen/api_v2#7061

Merged
MrAlias merged 6 commits intoopen-telemetry:mainfrom
javiermolinar:use-jaeger-idl
Apr 8, 2025
Merged

chore: use jaeger-idl/proto-gen/api_v2#7061
MrAlias merged 6 commits intoopen-telemetry:mainfrom
javiermolinar:use-jaeger-idl

Conversation

@javiermolinar
Copy link
Copy Markdown
Contributor

@javiermolinar javiermolinar commented Mar 28, 2025

This pr tries to fix a panic when jaegerremote is used along with the OTEL collector

panic: proto: duplicate enum registered: jaeger.api_v2.SamplingStrategyType
      goroutine 1 [running]:
      github.com/gogo/protobuf/proto.RegisterEnum(...)
       /races/vendor/github.com/gogo/protobuf/proto/properties.go:520
      github.com/jaegertracing/jaeger-idl/proto-gen/api_v2.init.4()
       /traces/vendor/github.com/jaegertracing/jaeger-idl/proto-gen/api_v2/sampling.pb.go:432 +0x219
      )

It also fixes the example by using the jaegerremotesampling extension instead of the deprecated remote_sampling

@javiermolinar javiermolinar requested review from a team and yurishkuro as code owners March 28, 2025 09:51
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Mar 28, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@javiermolinar
Copy link
Copy Markdown
Contributor Author

@javiermolinar
Copy link
Copy Markdown
Contributor Author

I've added a line in the changelog :)

@javiermolinar
Copy link
Copy Markdown
Contributor Author

Sorry for tagging you @yurishkuro but do we have an eta when this will be included in a new release? We are currently facing this problem, and I would like to get rid of the hacky solution we have. Thank you!

@yurishkuro
Copy link
Copy Markdown
Member

@javiermolinar I am neither maintainer nor approver in this repo, so cannot accelerate merging of the PR.

@open-telemetry/go-approvers can this be merged?

@javiermolinar
Copy link
Copy Markdown
Contributor Author

@MrAlias, @dmathieu Sorry for tagging both. Could you merge this? Thank you!

Copy link
Copy Markdown
Member

@dmathieu dmathieu left a comment

Choose a reason for hiding this comment

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

Could you fix the conflicts?

Comment thread CHANGELOG.md Outdated
@MrAlias MrAlias merged commit 56eeab7 into open-telemetry:main Apr 8, 2025
26 checks passed
@MrAlias MrAlias added this to the v1.36.0 milestone Apr 11, 2025
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.

4 participants