Skip to content

Conversation

@mabdinur
Copy link
Contributor

What does this PR do?

  • Updates the default value of DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED to True.

Motivation:

Ensures the full 128bit trace ids are injected into logs instead of just the lower order 64bits. This ensures the logs injected into traces is consistent with the trace id shown in the Datadog UI.

Change log entry

Updates the default value of DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED to True, ensuring logs contain the full 128-bit trace ID for consistency with the trace ID displayed in the Datadog UI.

Additional Notes:

How to test the change?

@mabdinur mabdinur requested a review from a team as a code owner March 19, 2025 20:13
@github-actions
Copy link

👋 Hey @DataDog/ruby-guild, please fill "Change log entry" section in the pull request description.

If changes need to be present in CHANGELOG.md you can state it this way

**Change log entry**

Yes. A brief summary to be placed into the CHANGELOG.md

(possible answers Yes/Yep/Yeah)

Or you can opt out like that

**Change log entry**

None.

(possible answers No/Nope/None)

Visited at: 2025-03-19 20:13:55 UTC

@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Mar 19, 2025

Datadog Report

Branch report: munir/enable-128bit-logging-by-default
Commit report: 5c65c0c
Test service: dd-trace-rb

✅ 0 Failed, 20779 Passed, 1373 Skipped, 3m 11.3s Total Time

@pr-commenter
Copy link

pr-commenter bot commented Mar 19, 2025

Benchmarks

Benchmark execution time: 2025-03-27 18:56:33

Comparing candidate commit 5c65c0c in PR branch munir/enable-128bit-logging-by-default with baseline commit a04dfda in branch master.

Found 0 performance improvements and 1 performance regressions! Performance is the same for 30 metrics, 2 unstable metrics.

scenario:tracing - Tracing.log_correlation

  • 🟥 throughput [-6793.391op/s; -6533.171op/s] or [-5.764%; -5.543%]

Copy link
Contributor

@TonyCTHsu TonyCTHsu left a comment

Choose a reason for hiding this comment

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

Changing default is not trivia, how does this change going to impact current users?

@mabdinur
Copy link
Contributor Author

Changing default is not trivia, how does this change going to impact current users?

This is a change we are rolling out across all client libraries. Ruby and PHP are the last set of libraries to receive this change. This change should not impact customers using Datadog log pipelines. Datadog log pipelines fully support correlating logs to traces using both 64bit and 128bit trace ids. This change could impact customers who export their logs to other providers. This is an edge case we are okay with.

@mabdinur mabdinur force-pushed the munir/enable-128bit-logging-by-default branch from c685ac2 to d685456 Compare March 21, 2025 14:59
@mabdinur mabdinur requested review from a team as code owners March 21, 2025 14:59
@mabdinur mabdinur requested a review from TonyCTHsu March 24, 2025 13:28
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2025

Codecov Report

Attention: Patch coverage is 97.36842% with 2 lines in your changes missing coverage. Please review.

Project coverage is 97.76%. Comparing base (a04dfda) to head (5c65c0c).
Report is 659 commits behind head on master.

Files with missing lines Patch % Lines
...rails/rails_semantic_logger_auto_injection_spec.rb 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4528      +/-   ##
==========================================
+ Coverage   97.75%   97.76%   +0.01%     
==========================================
  Files        1386     1392       +6     
  Lines       84299    84899     +600     
  Branches     4259     4279      +20     
==========================================
+ Hits        82404    83005     +601     
+ Misses       1895     1894       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mabdinur mabdinur requested review from marcotc and p-datadog March 27, 2025 17:07
@mabdinur mabdinur enabled auto-merge March 27, 2025 19:51
@mabdinur mabdinur dismissed TonyCTHsu’s stale review March 28, 2025 13:45

addressed concerns

@mabdinur mabdinur merged commit e239a85 into master Mar 28, 2025
460 checks passed
@mabdinur mabdinur deleted the munir/enable-128bit-logging-by-default branch March 28, 2025 13:45
@github-actions github-actions bot added this to the 2.13.0 milestone Mar 28, 2025
@vpellan vpellan mentioned this pull request Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants