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

editoast: 'OtelAxumLayer' act as 'INFO' level #10113

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

woshilapin
Copy link
Contributor

@woshilapin woshilapin commented Dec 17, 2024

By default, the 'OtelAxumLayer' only creates span at 'TRACE' level. That means that if you try tracing::Span::current() in a handler, you will get a Span::none() in response. This span is useless (it doesn't have a subscriber, no metadata): it can't be used with OpenTelemetry since all interactions between 'tracing' and 'opentelemetry' happens through Span::with_subscriber().

@woshilapin woshilapin requested a review from a team as a code owner December 17, 2024 15:23
@woshilapin woshilapin requested a review from leovalais December 17, 2024 15:23
@github-actions github-actions bot added the area:editoast Work on Editoast Service label Dec 17, 2024
@codecov-commenter
Copy link

codecov-commenter commented Dec 17, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.81%. Comparing base (844c6b3) to head (a10bb7a).
Report is 27 commits behind head on dev.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #10113      +/-   ##
==========================================
- Coverage   79.81%   79.81%   -0.01%     
==========================================
  Files        1053     1053              
  Lines      105483   105485       +2     
  Branches      727      727              
==========================================
- Hits        84196    84188       -8     
- Misses      21245    21255      +10     
  Partials       42       42              
Flag Coverage Δ
editoast 73.71% <ø> (-0.04%) ⬇️
front 89.13% <ø> (+<0.01%) ⬆️
gateway 2.18% <ø> (ø)
osrdyne 3.28% <ø> (ø)
railjson_generator 87.50% <ø> (ø)
tests 87.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

By default, the 'AxumOtelLayer' only creates span at 'TRACE' level.
That means that if you try `tracing::Span::current()` in a handler,
you will get a `Span::none()` in response. This span is useless (it
doesn't have a subscriber, no metadata): it can't be used with
OpenTelemetry since all interactions between 'tracing' and 'opentelemetry'
happens through `Span::with_subscriber()`.

Signed-off-by: Jean SIMARD <[email protected]>
Copy link
Contributor

@leovalais leovalais left a comment

Choose a reason for hiding this comment

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

LGTM.


As discussed yesterday, I wonder if we should really rely on trace IDs for our non logging-related features. I believe editoast should behave the same way regardless of the current RUST_LOG value. As you said, a middleware ensuring a trace ID is created could fix this issue.

@woshilapin
Copy link
Contributor Author

As discussed yesterday, I wonder if we should really rely on trace IDs for our non logging-related features. I believe editoast should behave the same way regardless of the current RUST_LOG value. As you said, a middleware ensuring a trace ID is created could fix this issue.

I agree. But in the case of STDCM debug logging, it's actually a logging functionality so I find it fine to tie this functionality to the logging framework. Actually, the STDCM debug logging functionality is configured (activated or not) through RUST_LOG.

That said, I believe the current PR has not relation with the STDCM logging functionality, we should not create traces only at TRACE level for our endpoints (or at least, I'm waiting for someone to come explain to me why it would make sense).

@woshilapin woshilapin added this pull request to the merge queue Dec 19, 2024
Merged via the queue into dev with commit 1033678 Dec 19, 2024
27 checks passed
@woshilapin woshilapin deleted the wsl/editoast/otel-layer-info branch December 19, 2024 21:39
@woshilapin woshilapin changed the title editoast: 'AxumOtelLayer' act as 'INFO' level editoast: 'OtelAxumLayer' act as 'INFO' level Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:editoast Work on Editoast Service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants