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

add a log propagator to allow correlation of a trace with logs #396

Merged
merged 2 commits into from
Dec 19, 2018

Conversation

rochdev
Copy link
Member

@rochdev rochdev commented Dec 17, 2018

This PR adds a propagator to allow correlation of a trace/span with logs. The propagator can be used with tracer.inject() like other propagators. The idea behind using a propagator is that a log entry is a carrier similar to an HTTP header, and the user should not have to worry about how to format the log entry themselves.

In the rare case where they are not using JSON and need full control on the log format, it's also already possible to access the IDs on the span context by using span.context().toTraceId() and span.context().toSpanId().

@rochdev rochdev added enhancement New feature or request core labels Dec 17, 2018
@rochdev rochdev added this to the 0.8.0 milestone Dec 17, 2018
@rochdev rochdev changed the base branch from master to v0.8.0 December 17, 2018 19:22
brettlangdon
brettlangdon previously approved these changes Dec 18, 2018
@rochdev
Copy link
Member Author

rochdev commented Dec 19, 2018

Updated to use span context methods instead of exposing private properties.

@brettlangdon Please re-approve whenever you can :)

Copy link
Member

@brettlangdon brettlangdon left a comment

Choose a reason for hiding this comment

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

minor semantic suggestion, otherwise this is fine

src/opentracing/span_context.js Show resolved Hide resolved
Copy link
Member

@brettlangdon brettlangdon left a comment

Choose a reason for hiding this comment

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

OT :(

👍

@rochdev rochdev merged commit c777e5b into v0.8.0 Dec 19, 2018
@rochdev rochdev deleted the log-correlation branch December 19, 2018 19:31
rochdev added a commit that referenced this pull request Feb 6, 2019
* v0.8.0

* prefix span context properties with an underscore (#397)

* add log propagator and trace identifiers for log correlation (#396)

* fix baggage items at the span level instead of trace level (#398)

* add middleware and stack trace support for express (#399)

* add dns integration (#405)

* add net integration (#406)

* add trace search sampling configuration (#407)

* add more metadata to net.connect and update operation name (#409)

* add automatic log correlation of tracer identifiers for winston (#408)

* add automatic log correlation of trace identifiers for bunyan (#410)

* add noop span context with the correct API (#413)

* add automatic log correlation of tracer identifiers for pino (#414)

* add automatic log correlation of tracer identifiers for pino

* add missing plugins to the build

* Add protocol as a configuration option for the dd-trace-agent URL (#416)

* Add protocol as a configuration option for the dd-trace agent URL.

This is useful for scenarios where a SSL-terminating load balancer sits in front of the datadog-agent

* Fix lint errors

* Switching from protocol override to url override per CR

* Fixing missing comma from previous commit

* Fix tests

* overrideUrl -> url

* add support for latest module versions in all plugins (#417)

* fix using log correlation with a custom logger (#419)

* disable net and dns plugins (#421)

* fix http server response handlers not running in the request scope (#422)

* fix log injection with a null active span and update injection keys (#423)

* Revert "add trace search sampling configuration (#407)" (#425)

This reverts commit 4dac394.

* add documentation about enabling log correlation (#427)

* fix references to the old trace IDs (#426)

* fix noop span context trace IDs to match the backend expectation (#429)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants