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.

Sign up for GitHub

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

Jump to bottom

RUMM-2718 Automatic otel b3 headers injection #1061

Merged
merged 10 commits into from Dec 2, 2022
Merged

RUMM-2718 Automatic otel b3 headers injection #1061

merged 10 commits into from Dec 2, 2022

Conversation

maciejburda
Copy link
Contributor

@maciejburda maciejburda commented Nov 23, 2022 •
edited

What and why?

Adds SDK wide configuration for automatic OTel support.

How?

Adds configuration enum and leverages mechanisms added in the previous PR.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • Run unit tests
  • Run integration tests
  • Run smoke tests

@maciejburda maciejburda requested a review from a team as a code owner November 23, 2022 14:12
@maciejburda maciejburda changed the title RUMM-2718 Add configuration for automatic open telemetry support RUMM-2718 Automatic Otel B3 Headers Injection Nov 23, 2022
@maciejburda maciejburda changed the title RUMM-2718 Automatic Otel B3 Headers Injection RUMM-2718 Automatic otel b3 headers injection Nov 23, 2022
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Nov 23, 2022 •
edited

Datadog Report

Branch report: maciey/RUMM-2718-automatic-otel-b3-headers-setup
Commit report: af0c8db

✅ dd-sdk-ios 0 Failed, 0 New Flaky, 112 Passed, 0 Skipped, 2m 35.17s Wall Time

@maciejburda maciejburda force-pushed the maciey/RUMM-2718-automatic-otel-b3-headers-setup branch 4 times, most recently from bd4397b to fc62b79 Compare November 24, 2022 13:43
maxep
maxep reviewed Nov 25, 2022
View reviewed changes
Copy link
Member

@maxep maxep left a comment

Choose a reason for hiding this comment

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

Nice work, it's very clear and I like the ability to use both header type 👍 I left minor suggestions.

Sources/Datadog/Core/DD/TracingHeaderType.swift Outdated Show resolved Hide resolved
Sources/Datadog/Tracing/Propagation/TracePropagationHeadersExctractor.swift Outdated Show resolved Hide resolved
Sources/DatadogObjc/DatadogConfiguration+objc.swift Outdated Show resolved Hide resolved
@maciejburda maciejburda force-pushed the maciey/RUMM-2718-automatic-otel-b3-headers-setup branch from fc62b79 to 2d173f4 Compare November 25, 2022 15:16
@maciejburda maciejburda force-pushed the maciey/RUMM-2718-automatic-otel-b3-headers-setup branch from b62a130 to b9178f2 Compare November 28, 2022 11:05
@maciejburda
Copy link
Contributor Author

maciejburda commented Nov 28, 2022 •
edited

@maxep @ncreated Ready for the second round of reviews 🙏

ncreated
ncreated requested changes Nov 29, 2022
View reviewed changes
Copy link
Collaborator

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

Really, well done 💪🏅. I left few comments, mainly on the public APIs and their comments. I'd like we first discuss it before merge. Implementation looks great 💯.

Datadog/Datadog.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
Datadog/Datadog.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved
Sources/Datadog/DatadogConfiguration.swift Outdated Show resolved Hide resolved
Sources/Datadog/Tracing/AutoInstrumentation/TracingHeaderType.swift Outdated Show resolved Hide resolved
@maciejburda maciejburda merged commit 4b1dcf3 into develop Dec 2, 2022
@maciejburda maciejburda deleted the maciey/RUMM-2718-automatic-otel-b3-headers-setup branch December 2, 2022 10:54
@maciejburda maciejburda mentioned this pull request Dec 2, 2022
Merged
6 tasks
@ncreated ncreated mentioned this pull request Dec 7, 2022
Merged
6 tasks
@maciejburda maciejburda mentioned this pull request Dec 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxep maxep maxep left review comments

@ncreated ncreated ncreated approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@maciejburda @ncreated @maxep