Skip to content

Conversation

@rhafer
Copy link
Contributor

@rhafer rhafer commented Sep 1, 2025

This PR has some smaller improvement for tracing

  • use otelchi middleware in the graph service
  • avoid unneeded extra span just for the request id attribute, by just adding it to the otelhttp created span
  • create spans for the various proxy middlewares.

I am still somewhat unhappy with how the span for the middlewares are created. Each middleware's span is the child span for the middleware that is before it in the chain. I'd prefer if they were just childs of the top-level span created by the otel framework. I might be missing something obvious, but it seems really clumsy to create the spans that way because of the way how they are passed via context.

@rhafer rhafer requested a review from butonic September 1, 2025 15:26
@rhafer rhafer self-assigned this Sep 1, 2025
@rhafer rhafer marked this pull request as draft September 1, 2025 15:29
@rhafer rhafer marked this pull request as ready for review September 2, 2025 10:04
Just add the request id as an attribute to the span created by the
'otelhttp' middleware.
Copy link
Contributor

@fschade fschade left a comment

Choose a reason for hiding this comment

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

optional

fmt.Sprintf("%s %v", req.Method, req.URL.Path)

could be

fmt.Sprintf("%s %s", req.Method, req.URL.Path)

@rhafer rhafer force-pushed the tracing-improvements branch from 496ad82 to 00c8029 Compare September 2, 2025 14:28
Each middleware adds a new span with a useful name now.
The graph service uses the `otelchi` middleware now to get at least some
basic tracing enabled.
@rhafer rhafer force-pushed the tracing-improvements branch from 00c8029 to ab6c39e Compare September 2, 2025 15:03
@rhafer rhafer merged commit b7a7804 into opencloud-eu:main Sep 2, 2025
54 checks passed
@openclouders openclouders mentioned this pull request Sep 2, 2025
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants