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-295 Add logs to spans #117

Merged
merged 5 commits into from
Jun 4, 2020
Merged

Conversation

ncreated
Copy link
Member

What and why?

📦 With this PR it is possible to send logs from a span:

let dataDownloadingSpan = tracer.startSpan(
    operationName: "data downloading",
    childOf: viewAppearingSpan.context
)

// Simulate logging download progress
dataDownloadingSpan.log(
    fields: [
        "message": "download progress",
        "progress": 0.99
    ]
)

dataDownloadingSpan.finish()

Screenshot 2020-05-27 at 11 06 57

How?

Technically, logs are linked to the span by the server, when this two internal attributes are provided:

static let traceID = "dd.trace_id"
static let spanID = "dd.span_id"

With small refactoring, I was able to fully reuse the Logging feature from DDTracer 🏅. The thin features integration layer was introduced:

internal struct TracingToLoggingOutput {
    /// `LogOutput` provided by the `Logging` feature.
    let loggingOutput: LogOutput

    func writeLogWith(spanContext: DDSpanContext, fields: [String: Encodable], date: Date) {
       // ... prepare the log ...
       loggingOutput.writeLogWith(level: .info, message: message, date: date, attributes: logAttributes, tags: [])
    }
}

I've put this TracingToLoggingOutput in FeaturesIntegration group, so we separate both features on the project level:

Screenshot 2020-05-27 at 11 15 41

Because now the Tracing feature uses the storage stack from the Logging feature, it was reflected in the TracingFeature initializer ✨:

// TracingFeature

init(
    directory: Directory,
    configuration: Datadog.ValidConfiguration,
    performance: PerformancePreset,
+    loggingFeatureStorage: LoggingFeature.Storage,
    mobileDevice: MobileDevice,
    httpClient: HTTPClient,
) { /* ... */ }

Refactoring

This integration required a refactoring of the Logging feature, so that the log date is no longer created by the LogBuilder. This responsibility was moved to the Logger. This is because the Open Tracing API enables the user to specify the log timestamp explicitly:

// OpenTracing.Span
func log(fields: [String: Codable], timestamp: Date)

This is even better for the overall Logging architecture, as now the Logger becomes a "thin facade" for preparing the log's traits before passing them further to the LogFileOutput for doing the "heavy" work.

Testing

Beside unit tests for "create log for Tracing", I also added required integration test. Now the "Send traces for UI Tests" fixture not only sends spans, but also adds log to the "data downloading". Both, spans and logs delivery to HTTPServerMock is asserted in TracingIntegrationTests 🚀.

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

Sorry, something went wrong.

ncreated added 4 commits May 26, 2020 11:35

Verified

This commit was signed with the committer’s verified signature.
jbaldwin Josh Baldwin

Verified

This commit was signed with the committer’s verified signature.
jbaldwin Josh Baldwin

Verified

This commit was signed with the committer’s verified signature.
jbaldwin Josh Baldwin

Verified

This commit was signed with the committer’s verified signature.
jbaldwin Josh Baldwin
@ncreated ncreated added this to the tracing milestone May 27, 2020
@ncreated ncreated requested a review from a team as a code owner May 27, 2020 09:34
@ncreated ncreated self-assigned this May 27, 2020
@ncreated ncreated requested a review from buranmert June 3, 2020 14:41
@ncreated ncreated requested a review from buranmert June 4, 2020 09:47
Copy link
Contributor

@buranmert buranmert left a comment

Choose a reason for hiding this comment

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

approved as #120 resolves the discussion here

@ncreated ncreated merged commit 111a94e into tracing Jun 4, 2020
@ncreated ncreated deleted the ncreated/RUMM-295-add-logs-to-spans branch June 4, 2020 12:05
@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.

None yet

2 participants