Skip to content

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

Merged
0xnm merged 1 commit into
feature/sdkv2from
nogorodnikov/rumm-2608/use-event-write-context-for-traces
Oct 27, 2022
Merged

RUMM-2608: Use event write context for Traces#1106
0xnm merged 1 commit into
feature/sdkv2from
nogorodnikov/rumm-2608/use-event-write-context-for-traces

Conversation

@0xnm

@0xnm 0xnm commented Oct 26, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

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

codecov-commenter commented Oct 26, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.87452% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.35%. Comparing base (9b73f57) to head (e8b3371).

⚠️ Current head e8b3371 differs from pull request most recent head 9720e9c

Please upload reports for the commit 9720e9c to get more accurate results.

Files with missing lines Patch % Lines
...in/com/datadog/android/log/internal/LogsFeature.kt 85.96% 2 Missing and 6 partials ⚠️
...og/android/v2/core/internal/NoOpContextProvider.kt 0.00% 6 Missing ⚠️
...g/android/log/internal/logger/DatadogLogHandler.kt 86.96% 1 Missing and 2 partials ⚠️
.../android/error/internal/DatadogExceptionHandler.kt 84.62% 1 Missing and 1 partial ⚠️
.../android/v2/log/internal/storage/LogsDataWriter.kt 60.00% 2 Missing ⚠️
.../src/main/kotlin/com/datadog/android/log/Logger.kt 93.33% 0 Missing and 1 partial ⚠️
...android/log/internal/domain/DatadogLogGenerator.kt 98.18% 0 Missing and 1 partial ⚠️
...tadog/android/tracing/internal/data/TraceWriter.kt 95.65% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@                Coverage Diff                @@
##           feature/sdkv2    #1106      +/-   ##
=================================================
- Coverage          82.69%   82.35%   -0.34%     
=================================================
  Files                350      351       +1     
  Lines              11301    11367      +66     
  Branches            1846     1878      +32     
=================================================
+ Hits                9345     9361      +16     
- Misses              1394     1432      +38     
- Partials             562      574      +12
Files with missing lines Coverage Δ
...n/com/datadog/android/core/internal/CoreFeature.kt 91.09% <ø> (-0.35%) ⬇️
...in/com/datadog/android/core/internal/SdkFeature.kt 88.18% <ø> (ø)
...adog/android/error/internal/CrashReportsFeature.kt 100.00% <100.00%> (ø)
...atadog/android/log/internal/domain/LogGenerator.kt 80.00% <ø> (-3.33%) ⬇️
...g/android/rum/internal/domain/scope/RumEventExt.kt 93.75% <100.00%> (ø)
...android/rum/internal/ndk/DatadogNdkCrashHandler.kt 86.32% <100.00%> (-1.51%) ⬇️
...otlin/com/datadog/android/tracing/AndroidTracer.kt 96.92% <100.00%> (+1.47%) ⬆️
...datadog/android/tracing/internal/TracingFeature.kt 100.00% <100.00%> (ø)
...atadog/android/tracing/internal/data/NoOpWriter.kt 100.00% <100.00%> (ø)
...g/internal/domain/event/DdSpanToSpanEventMapper.kt 100.00% <100.00%> (ø)
... and 12 more

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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
mariusc83
mariusc83 approved these changes Oct 27, 2022
View reviewed changes

@mariusc83 mariusc83 left a comment

Copy link
Copy Markdown
Member

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
This was referenced Oct 31, 2022
Merged
Merged
@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

Reviewers

@mariusc83 mariusc83 mariusc83 approved these changes
+1 more reviewer
@xgouchet xgouchet xgouchet approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size-medium This PR is medium sized

Projects

None yet

Milestone

1.16.0

Development

Successfully merging this pull request may close these issues.

4 participants

@0xnm @codecov-commenter @xgouchet @mariusc83