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

feat: Enable tracing #376

Merged
merged 1 commit into from
Mar 14, 2020
Merged

feat: Enable tracing #376

merged 1 commit into from
Mar 14, 2020

Conversation

ptescher
Copy link
Contributor

@ptescher ptescher commented Mar 9, 2020

Proposed changes

Enable tracing via ory/x/tracing just like Hydra etc.

Checklist

  • I have read the contributing guidelines
  • I have read the security policy
  • I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security
    vulnerability, I confirm that I got green light (please contact [email protected]) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation within the code base (if appropriate)
  • I have documented my changes in the developer guide (if appropriate)

@ptescher ptescher changed the title Enable tracing Feature/Enable tracing Mar 9, 2020
@ptescher ptescher changed the title Feature/Enable tracing feat: Enable tracing Mar 9, 2020
@aeneasr
Copy link
Member

aeneasr commented Mar 13, 2020

Cool! Could you maybe add a jaeger example to the docker compose file similar to the way we have done it for Hydra (see code)? That would be super helpful to check out the feature!

@ptescher
Copy link
Contributor Author

@aeneasr done!

@ptescher
Copy link
Contributor Author

image

@aeneasr
Copy link
Member

aeneasr commented Mar 14, 2020

Awesome, thank you!

@aeneasr aeneasr merged commit cb0f3f2 into ory:master Mar 14, 2020
@ptescher ptescher deleted the feature/enable-tracing branch March 16, 2020 16:02
@snikch
Copy link
Contributor

snikch commented Feb 19, 2021

FYI this capability isn't in the docs so it was a nice surprise to find this PR!

Confirming though, this doesn't add the spans to outgoing requests from Oathkeeper does it? i.e. it won't allow be to continue tracing through to Hydra.

@daviddelucca
Copy link
Contributor

@snikch does it work? I have set jaeger but I don't see oathkeeper requests to upstream or mutator

@snikch
Copy link
Contributor

snikch commented Sep 22, 2022

@daviddelucca works fine for us
image

@daviddelucca
Copy link
Contributor

@snikch

I'm just seeing request for introspection. Are you able to find tracing for upstream host requests?

@snikch
Copy link
Contributor

snikch commented Sep 26, 2022

@daviddelucca ah yes, correct. I added this separately here: #640

You'd need to have the equivalent code added to other endpoints I think. A bigger refactor would be to use a shared http client with tracing enabled so each separate area doesn't need to have it implemented.

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