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 tracing to exemplar APIs #4299

Merged
merged 5 commits into from
Jun 3, 2021

Conversation

bill3tt
Copy link
Contributor

@bill3tt bill3tt commented Jun 3, 2021

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Follows #4238 & #4178 - adds tracing instrumentation to the exemplar endpoints.

Closes #4130

Verification

Standard tests pass:

make build
make test-local

I have been trying and failing to figure out an additional unit test to verify this behaviour.

It might be possible to setup a grpc server with a basictracer-go and register the exemplar APIs then verify spans are created when the API is called. Previous tracing PRs have not included this so might not be worth the effort 🤷

bill3tt and others added 3 commits June 3, 2021 09:15
Signed-off-by: Ian Billett <ibillett@redhat.com>
Signed-off-by: Ian Billett <ibillett@redhat.com>
Signed-off-by: Ian Billett <ibillett@redhat.com>
Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

Looks great :) Only minor issues.

I have added a couple of comments inline.

pkg/api/query/v1.go Outdated Show resolved Hide resolved
pkg/api/query/v1.go Outdated Show resolved Hide resolved
Co-authored-by: Kemal Akkoyun <kakkoyun@users.noreply.github.com>
Signed-off-by: Ian Billett <ibillett@redhat.com>
@bill3tt bill3tt force-pushed the add-tracing-to-exemplar-endpoints branch from 0901471 to fb293b6 Compare June 3, 2021 09:52
Signed-off-by: Ian Billett <ibillett@redhat.com>
@bill3tt
Copy link
Contributor Author

bill3tt commented Jun 3, 2021

Think this puppy is ready to merge 💪

Copy link
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@kakkoyun kakkoyun merged commit e982ff8 into thanos-io:main Jun 3, 2021
@squat
Copy link
Member

squat commented Jun 3, 2021

nice :))

@bill3tt
Copy link
Contributor Author

bill3tt commented Jun 3, 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.

Add tracing for federated exemplar API
3 participants