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-409 Update tracing with service, env and version #107

Merged
merged 11 commits into from
May 14, 2020

Conversation

ncreated
Copy link
Member

@ncreated ncreated commented May 13, 2020

What and why?

🚚 In #102, service, env and version attributes were unified for Log. In this PR, I merge master to tracing by also applying the same things to Span.

How?

There are three groups of commits in this PR:

  • 3cc0bda - ca3cc71 - they update tracing with master, no CR required.
  • 38f769a - this is the merge commit where I solved all the git conflicts.
  • 4c2a1c1 - the most important commit, where Tracing feature gets updated with new service, env and version rules.

As discussed in #100, the default values resolution was moved from DDTracer.ResolvedConfiguration (deleted) to DDTracer itself. This was possible, as now we can make assertions on SpanBuilder:

func testDefaultTracer() {
    let tracer = DDTracer.initialize(configuration: .init()).dd

    let spanBuilder = (tracer.spanOutput as! SpanFileOutput)!.spanBuilder
    // ...
    XCTAssertEqual(spanBuilder.serviceName, "service-name")
    // ...
}

func testCustomizedTracer() {
    let tracer = DDTracer.initialize(
        configuration: .init(serviceName: "custom-service-name")
    ).dd

    let spanBuilder = (tracer.spanOutput as! SpanFileOutput)!.spanBuilder
    // ...
    XCTAssertEqual(spanBuilder.serviceName, "custom-service-name")
    // ...
}

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

ncreated added 10 commits May 8, 2020 15:44
…-env-and-version

RUMM-409 Unify `service`, `env` and `version` across all SDKs
# Conflicts:
#	Datadog/Datadog.xcodeproj/project.pbxproj
#	Shopist/Shopist/AppDelegate.swift
#	Sources/Datadog/Core/Upload/HTTPHeaders.swift
#	Sources/Datadog/Datadog.swift
#	Sources/Datadog/DatadogConfiguration.swift
#	Sources/Datadog/Logs/LoggingFeature.swift
#	Tests/DatadogIntegrationTests/Benchmark/BenchmarkTests.swift
#	Tests/DatadogIntegrationTests/LoggingIntegrationTests.swift
#	Tests/DatadogTests/Datadog/Core/Upload/DataUploaderTests.swift
#	Tests/DatadogTests/Datadog/Core/Upload/HTTPHeadersTests.swift
#	Tests/DatadogTests/Datadog/DatadogTests.swift
#	Tests/DatadogTests/Datadog/Mocks/DatadogMocks.swift
#	Tests/DatadogTests/Datadog/Mocks/DatadogObjcMocks.swift
#	Tests/DatadogTests/Datadog/Mocks/LoggingFeatureMocks.swift
@ncreated ncreated added this to the tracing milestone May 13, 2020
@ncreated ncreated self-assigned this May 13, 2020
@ncreated ncreated changed the title RUMM-409 Update tracing with service, env and version from #102 RUMM-409 Update tracing with service, env and version May 13, 2020
@ncreated ncreated marked this pull request as ready for review May 13, 2020 12:08
@ncreated ncreated requested a review from a team as a code owner May 13, 2020 12:08
@buranmert
Copy link
Contributor

  1. In order to create DDTracer, we need TracingFeature
  2. In order to create TracingFeature, we need Datadog.instance
  3. Datadog.instance has the default configuration in it

Is this order correct? Is that how we could get rid of resolve(config: ...)?

@ncreated
Copy link
Member Author

  1. In order to create DDTracer, we need TracingFeature

Yes 👍.

  1. In order to create TracingFeature, we need Datadog.instance

No. TracingFeature only needs the value of struct Datadog.ValidConfiguration. This way we don't need mocking the Datadog.instance in tests that require TracingFeature.instance.

  1. Datadog.instance has the default configuration in it

It only takes the configuration from the user: Datadog.Configuration, then resolves it with AppContext and creates the value of Datadog.ValidConfiguration.

It doesn't retain Datadog.ValidConfiguration. It just creates it and passes it to features, so it is accessible through:

  • LoggingFeature.instance.configuration
  • and TracingFeature.instance.configuration.

@ncreated ncreated merged commit ba8b4c0 into tracing May 14, 2020
@ncreated ncreated deleted the ncreated/RUMM-409-merge-master-to-tracing branch May 14, 2020 17:07
@ncreated ncreated modified the milestones: tracing, next-version Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants