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 request id to important http handlers #3376

Merged
merged 1 commit into from
Oct 24, 2022
Merged

Conversation

micbar
Copy link
Member

@micbar micbar commented Oct 19, 2022

Add request-id to logs in

  • ocdav
  • ocs
  • dataprovider
  • archiver

@update-docs
Copy link

update-docs bot commented Oct 19, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

Copy link
Contributor

@rhafer rhafer left a comment

Choose a reason for hiding this comment

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

I am NOT completely against this, but wouldn't it be a lot cleaner to add the middleware in the same way how all the other middlewares are handled in pkg/rhttp/rhttp.go? Instead of touching every single service individually?

@micbar micbar force-pushed the requestID branch 2 times, most recently from 1dfc6a7 to c044d0c Compare October 21, 2022 12:37
@@ -191,8 +192,11 @@ func useMiddlewares(r *chi.Mux, sopts *Options, svc global.Service, tp trace.Tra
// ctx
cm := appctx.New(sopts.Logger, tp)

// request-id
rm := middleware.RequestID
Copy link
Member Author

Choose a reason for hiding this comment

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

@rhafer Could not find another way. Any ideas? the ocdav service starts differently

@micbar micbar requested a review from rhafer October 21, 2022 12:38
@micbar micbar merged commit e470acc into cs3org:edge Oct 24, 2022
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.

3 participants