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: add datadog and opentelemetry #6677

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

woshilapin
Copy link
Contributor

@woshilapin woshilapin commented Feb 20, 2024

Following up on #6649 and #6650, editoast now can report all traces events to Opentelemetry or Datadog depending on runtime configuration.

Ready for review since I was able to observe traces on local for Opentelemetry (launching a local Jaeger to consume it) and Datadog (launching a local Datadog agent and looking at the logs).

Screenshot of existing traces on Jaeger

image

Logs from a Datadog agent

Look at the traces received in the following log.

❯  docker logs -f datadog-agent
[...]
2024-03-06 15:52:48 UTC | TRACE | INFO | (pkg/trace/info/stats.go:98 in LogAndResetStats) | [lang:rust tracer_version:0.9.0 endpoint_version:v0.5] -> traces received: 10, traces filtered: 0, traces amount: 2602 bytes, events extracted: 0, events sampled: 0
⚠️ This was not yet tested in a real deployed environment, test still on-going on that part, but I believe the current code is already in a good state for a first review.

Closes #6660.

Copy link

codecov bot commented Feb 20, 2024

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 28.26%. Comparing base (629612c) to head (821fe41).

Files Patch % Lines
editoast/src/client/telemetry_config.rs 0.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #6677      +/-   ##
============================================
- Coverage     28.29%   28.26%   -0.03%     
  Complexity     2179     2179              
============================================
  Files          1052     1053       +1     
  Lines        130736   130813      +77     
  Branches       2602     2602              
============================================
- Hits          36987    36979       -8     
- Misses        92236    92321      +85     
  Partials       1513     1513              
Flag Coverage Δ
core 79.44% <ø> (ø)
editoast 74.81% <0.00%> (-0.28%) ⬇️
front 8.74% <ø> (ø)
gateway 2.50% <ø> (ø)
railjson_generator 87.26% <ø> (ø)
tests 83.07% <ø> (ø)

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.

@woshilapin woshilapin force-pushed the wsl/feat/datadog-opentelemetry branch from 6eff2bb to 2e8bbe2 Compare February 20, 2024 14:41
@woshilapin woshilapin force-pushed the wsl/feat/datadog-opentelemetry branch from 2e8bbe2 to 075c791 Compare March 6, 2024 15:55
@woshilapin woshilapin requested review from leovalais and hamz2a March 6, 2024 15:56
@woshilapin woshilapin marked this pull request as ready for review March 6, 2024 15:56
@woshilapin woshilapin requested a review from a team as a code owner March 6, 2024 15:56
Copy link
Contributor

@hamz2a hamz2a left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor

@flomonster flomonster left a comment

Choose a reason for hiding this comment

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

I made a few request changes but it looks good to me.
I failed to see the tracks editoast in jaegger however.
It may be due to misuse on my part.

Copy link
Contributor

@flomonster flomonster left a comment

Choose a reason for hiding this comment

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

Nevermind it's working 🎉 We should enable open telemetry by default using docker.

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, not tested. thanks! 🚀

@woshilapin
Copy link
Contributor Author

I also defaulted to activate Opentelemetry by default (like in gateway). See 00cdd7c.

@woshilapin woshilapin force-pushed the wsl/feat/datadog-opentelemetry branch from 00cdd7c to 3b9367c Compare March 7, 2024 16:03
@woshilapin woshilapin enabled auto-merge March 7, 2024 16:04
Following up on #6649 and #6650, `editoast` now can
report all traces events to Opentelemetry or Datadog
depending on runtime configuration.
@woshilapin woshilapin force-pushed the wsl/feat/datadog-opentelemetry branch from 3b9367c to 821fe41 Compare March 7, 2024 16:16
@woshilapin woshilapin added this pull request to the merge queue Mar 7, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 7, 2024
@woshilapin woshilapin added this pull request to the merge queue Mar 7, 2024
Merged via the queue into dev with commit ae6706b Mar 7, 2024
22 checks passed
@woshilapin woshilapin deleted the wsl/feat/datadog-opentelemetry branch March 7, 2024 17:33
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.

Send OTEL from editoast to Datadog
4 participants