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

RUMM-2608: Use event write context for Traces #1106

Merged

Conversation

0xnm
Copy link
Member

@0xnm 0xnm commented Oct 26, 2022

What does this PR do?

This PR brings the usage of event write context (which can be retrieved by calling withWriteContext) for traces.

It is done by changing the TraceWriter so that it requests event write context for the collection of DDSpan to write.

EWC will be requests only once, so that DD context is the same for all the spans to be written.

Also introduced ContextAwareMapper and ContextAwareSerializer, because both mapping (from legacy DDSpan to SpanEvent and serialization) require DatadogContext access, and options like to create them each time inside EWC or use Pair<DatadogContext, T> as argument seemed less reasonable to me.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@0xnm 0xnm requested a review from a team as a code owner October 26, 2022 14:38
@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2022

Codecov Report

Merging #1106 (e8b3371) into feature/sdkv2 (e201ff4) will decrease coverage by 0.29%.
The diff coverage is 90.87%.

❗ Current head e8b3371 differs from pull request most recent head 9720e9c. Consider uploading reports for the commit 9720e9c to get more accurate results

@@                Coverage Diff                @@
##           feature/sdkv2    #1106      +/-   ##
=================================================
- Coverage          82.64%   82.35%   -0.29%     
=================================================
  Files                350      351       +1     
  Lines              11301    11367      +66     
  Branches            1846     1878      +32     
=================================================
+ Hits                9339     9361      +22     
- Misses              1396     1432      +36     
- Partials             566      574       +8     
Impacted Files Coverage Δ
...n/com/datadog/android/core/internal/CoreFeature.kt 91.09% <ø> (-0.35%) ⬇️
...in/com/datadog/android/core/internal/SdkFeature.kt 88.18% <ø> (ø)
...atadog/android/log/internal/domain/LogGenerator.kt 80.00% <ø> (-3.33%) ⬇️
...og/android/v2/core/internal/NoOpContextProvider.kt 3.57% <0.00%> (-0.78%) ⬇️
.../android/v2/log/internal/storage/LogsDataWriter.kt 60.00% <60.00%> (ø)
.../android/error/internal/DatadogExceptionHandler.kt 81.08% <84.62%> (-4.63%) ⬇️
...in/com/datadog/android/log/internal/LogsFeature.kt 87.10% <85.96%> (-12.90%) ⬇️
...g/android/log/internal/logger/DatadogLogHandler.kt 90.91% <86.96%> (-0.76%) ⬇️
.../src/main/kotlin/com/datadog/android/log/Logger.kt 94.02% <93.33%> (+0.12%) ⬆️
...tadog/android/tracing/internal/data/TraceWriter.kt 95.83% <95.65%> (-4.17%) ⬇️
... and 26 more

@xgouchet xgouchet added the size-medium This PR is medium sized label Oct 26, 2022
@0xnm 0xnm changed the title RUMM-2608: Use event write context for traces RUMM-2608: Use event write context for Traces Oct 26, 2022
Copy link
Member

@mariusc83 mariusc83 left a comment

Choose a reason for hiding this comment

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

Nicely done. Lgtm !!

@0xnm 0xnm force-pushed the nogorodnikov/rumm-2607/use-eventwritecontext-for-logs branch from cd86390 to 169cbec Compare October 27, 2022 11:44
Base automatically changed from nogorodnikov/rumm-2607/use-eventwritecontext-for-logs to feature/sdkv2 October 27, 2022 12:36
@0xnm 0xnm force-pushed the nogorodnikov/rumm-2608/use-event-write-context-for-traces branch from e8b3371 to 9720e9c Compare October 27, 2022 12:43
@0xnm 0xnm merged commit 86c8687 into feature/sdkv2 Oct 27, 2022
@0xnm 0xnm deleted the nogorodnikov/rumm-2608/use-event-write-context-for-traces branch October 27, 2022 13:04
@xgouchet xgouchet added this to the 1.16.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size-medium This PR is medium sized
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants