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.

Sign up for GitHub

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

Jump to bottom

Update Jaeger model import paths to use jaeger-idl v1 #37652

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Update Jaeger model import paths to use jaeger-idl v1 #37652

wants to merge 4 commits into from
+78 −54

Conversation

Nabil-Salah
Copy link

Description

Update jaeger models

Link to tracking issue

jaegertracing/jaeger#6494

Testing

go test in each updated module

Documentation

@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Feb 3, 2025 •
edited
Loading

CLA Not Signed

@Nabil-Salah
Copy link
Author

hello @yurishkuro
I tried this make the draft pr and tested every module with go test and they pass without any problem so it looks promising.

@yurishkuro
Copy link
Member

That's surprising that you were able to do that without bumping the actual jaeger dependency. Did you modify all places?

You need to run make gotidy or something like that to update go.sum

@Nabil-Salah
Copy link
Author

Nabil-Salah commented Feb 4, 2025 •
edited
Loading

Did you modify all places?

yes these are all the places for the model not for proto-gen or thrift-gen
but if models succeeds I can't think of a problem in the other two

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aneurysm9 Aneurysm9 Awaiting requested review from Aneurysm9

@atoulme atoulme Awaiting requested review from atoulme atoulme is a code owner

@dao-jun dao-jun Awaiting requested review from dao-jun

@dmitryax dmitryax Awaiting requested review from dmitryax dmitryax is a code owner

@frzifus frzifus Awaiting requested review from frzifus

@MovieStoreGuy MovieStoreGuy Awaiting requested review from MovieStoreGuy MovieStoreGuy is a code owner

@pavolloffay pavolloffay Awaiting requested review from pavolloffay

@VihasMakwana VihasMakwana Awaiting requested review from VihasMakwana

@yotamloe yotamloe Awaiting requested review from yotamloe

@yurishkuro yurishkuro Awaiting requested review from yurishkuro yurishkuro is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
exporter/awskinesis exporter/kafka exporter/logzio exporter/pulsar exporter/sapm extension/encoding pkg/translator/jaeger receiver/jaeger receiver/kafka receiver/pulsar receiver/sapm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Nabil-Salah @yurishkuro