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: Decorate request logs with trace_id and span_id fields #445

Conversation

claudio-benfatto
Copy link
Contributor

@claudio-benfatto claudio-benfatto commented May 22, 2020

Related issue

Proposed changes

This PR introduces opentracing, trace_id and span_id request tracing fields to the api and proxy handlers.
The implementation is very similar to: ory/hydra#1685.

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 or changed the documentation.

Further comments

In this PR I opted for decorating the logs with a private method in the server package. However, and given the fact that this is already the second project (up to my knowledge) introducing this feature, would it make sense to add the code to the reqlog package

@aeneasr
Copy link
Member

aeneasr commented May 23, 2020

Thank you for your work on this! I'd like to take this but I want to move it to another place because we're currently refactoring the logger. Is it ok if I keep this open and integrate this work into our new logging component?

@claudio-benfatto claudio-benfatto force-pushed the feat-enable_opetracing_fields_in_logs branch from 6b4dd0e to 56f2fce Compare May 24, 2020 11:01
@claudio-benfatto
Copy link
Contributor Author

Thank you for your work on this! I'd like to take this but I want to move it to another place because we're currently refactoring the logger. Is it ok if I keep this open and integrate this work into our new logging component?

Hi @aeneasr of course yes, no problem at all! Thanks!

@aeneasr aeneasr self-assigned this Apr 6, 2021
@aeneasr
Copy link
Member

aeneasr commented Apr 23, 2021

Sorry that I didn't have time to work on this. Cleaning up old issues and closing this one but keeping it tracked for #441

@aeneasr aeneasr closed this Apr 23, 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.

2 participants