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

RUM-1836 feat(otel-tracer) integrate OpenTelemetrySwiftApi in package managers #1668

Merged
merged 1 commit into from
Feb 15, 2024
Merged

RUM-1836 feat(otel-tracer) integrate OpenTelemetrySwiftApi in package managers #1668

merged 1 commit into from
Feb 15, 2024

Conversation

ganeshnj
Copy link
Contributor

@ganeshnj ganeshnj commented Feb 9, 2024 •
edited
Loading

What and why?

Otel APIs must work on all packages manager setups.

https://datadoghq.atlassian.net/browse/RUM-2731
https://datadoghq.atlassian.net/browse/RUM-2732
https://datadoghq.atlassian.net/browse/RUM-2733

How?

This PR introduces mechanism to consume the Trace package with pure xcframework, CP, Carthage and SPM (already added). This is done via shipping the OpenTelemetryApi package as a standalone package in all three formats. Its release are managed at https://github.com/DataDog/opentelemetry-swift-packages

https://datadoghq.atlassian.net/browse/RUM-3185 is created to re-enable the CI visibility.

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 for Core, RUM, Trace, Logs, CR and WVT
  • Run unit tests for Session Replay
  • Run integration tests
  • Run smoke tests
  • Run tests for tools/

@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Feb 9, 2024 •
edited
Loading

Datadog Report

Branch report: ganeshnj/otel-tracer-deps
Commit report: 082ded4
Test service: dd-sdk-ios

❌ 8 Failed (0 Known Flaky), 1222 Passed, 0 Skipped, 3m 32.87s Wall Time
❄️ 1 New Flaky
🔻 Test Sessions change in coverage: 6 decreased

❌ Failed Tests (8)

This report shows up to 5 failed tests.

  • testToDatadog - OTelSpanIdDatadogTests - Details

    Expand for error 
     Assertion Failure at OTelSpanId+DatadogTests.swift:18: XCTAssertEqual failed: ("6103117552") is not equal to ("4561313280")

  • testEncoder_givenAllPropertiesArePresent - OTelSpanLinkTests - Details

    Expand for error 
     Segmentation fault. Check error.crash_log for the full crash log.

  • testEncoder_givenOnlyRequiredPropertiesArePresent - OTelSpanLinkTests - Details

    Expand for error 
     Segmentation fault. Check error.crash_log for the full crash log.

  • testSpanEnd - OTelSpanTests - Details

    Expand for error 
     Bus error. Check error.crash_log for the full crash log.

  • testToDatadog_onlyHigherOrderBitsAreConsidered - OTelTraceIdDatadogTests - Details

    Expand for error 
     Assertion Failure at OTelTraceId+DatadogTests.swift:18: XCTAssertEqual failed: ("6128660208") is not equal to ("4535185152")

New Flaky Tests (1)

  • testWhenCPUUnderHeavyLoadItMeasuresHigherCPUTicks - VitalCPUReaderTest - Last Failure

    Expand for error 
     Assertion Failure at VitalCPUReaderTests.swift:24: XCTAssertGreaterThan failed: ("229.17755849427255") is not greater than ("231.5078193604381")

🔻 Code Coverage Decreases vs Default Branch (6)

This report shows up to 5 code coverage decreases.

  • test DatadogTraceTests iOS 39.06% (-10.15%) - Details
  • test DatadogLogsTests iOS 44.38% (-0.51%) - Details
  • test DatadogWebViewTrackingTests iOS 22.69% (-0.38%) - Details
  • test DatadogInternalTests iOS 80.97% (-0.26%) - Details
  • test DatadogRUMTests iOS 79.21% (-0.20%) - Details

ganeshnj
ganeshnj commented Feb 13, 2024
View reviewed changes
Cartfile
@@ -1 +1,2 @@
github "microsoft/plcrashreporter" ~> 1.11.1
binary "https://raw.githubusercontent.com/DataDog/opentelemetry-swift-packages/ganeshnj/ci/pod-push/OpenTelemetryApi.json" ~> 1.9.1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will be changed to main branch once we have the changes merged.

ganeshnj
ganeshnj commented Feb 13, 2024
View reviewed changes
Makefile
@@ -2,6 +2,7 @@ all: dependencies templates

# The release version of `dd-sdk-swift-testing` to use for tests instrumentation.
DD_SDK_SWIFT_TESTING_VERSION = 2.3.2
DD_DISABLE_TEST_INSTRUMENTING = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

had to disable for the moment

backlog RUM-3185

Copy link
Member

Choose a reason for hiding this comment

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

probably worth having a code comment with ticket reference as well

Copy link
Member

Choose a reason for hiding this comment

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

suggestion/ The RUM-3185 gives enough context on why we disable it 👍. Let's include that JIRA in the PR description, so it is clear why we had to turn it off.

@ganeshnj ganeshnj marked this pull request as ready for review February 13, 2024 11:59
@ganeshnj ganeshnj requested review from a team as code owners February 13, 2024 11:59
maciejburda
maciejburda approved these changes Feb 15, 2024
View reviewed changes
Makefile
@@ -2,6 +2,7 @@ all: dependencies templates

# The release version of `dd-sdk-swift-testing` to use for tests instrumentation.
DD_SDK_SWIFT_TESTING_VERSION = 2.3.2
DD_DISABLE_TEST_INSTRUMENTING = true
Copy link
Member

Choose a reason for hiding this comment

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

probably worth having a code comment with ticket reference as well

ncreated
ncreated approved these changes Feb 15, 2024
View reviewed changes
Copy link
Member

@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.

Looks good 👍, I left one small suggestion.

Makefile
@@ -2,6 +2,7 @@ all: dependencies templates

# The release version of `dd-sdk-swift-testing` to use for tests instrumentation.
DD_SDK_SWIFT_TESTING_VERSION = 2.3.2
DD_DISABLE_TEST_INSTRUMENTING = true
Copy link
Member

Choose a reason for hiding this comment

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

suggestion/ The RUM-3185 gives enough context on why we disable it 👍. Let's include that JIRA in the PR description, so it is clear why we had to turn it off.

@ganeshnj ganeshnj force-pushed the ganeshnj/otel-tracer-deps branch from c3db3a0 to 1a82df3 Compare February 15, 2024 12:15
@ganeshnj ganeshnj merged commit f605c7c into ganeshnj/feat/otel-tracer Feb 15, 2024
5 of 10 checks passed
@ganeshnj ganeshnj deleted the ganeshnj/otel-tracer-deps branch February 15, 2024 13:04
@ncreated ncreated mentioned this pull request May 31, 2024
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maciejburda maciejburda maciejburda approved these changes

@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.
3 participants
@ganeshnj @ncreated @maciejburda