Skip to content

Automatic Correlation Identifier Injection: Add properties as strings instead of longs#528

Merged
zacharycmontoya merged 5 commits intomasterfrom
zach/fixes/correlation-identifier-string
Oct 17, 2019
Merged

Automatic Correlation Identifier Injection: Add properties as strings instead of longs#528
zacharycmontoya merged 5 commits intomasterfrom
zach/fixes/correlation-identifier-string

Conversation

@zacharycmontoya
Copy link
Contributor

We currently save the dd.trace_id and dd.span_id values in the logging library's logging context as long values. This causes an issue when producing JSON formatted output for Serilog because the resulting values are integers, not strings. This PR changes our automatic correlation identifier injection to store the values as string values instead of long values.

Additionally, re-write the unit tests for each logging library to test that JSON formatting results in string values.

@DataDog/apm-dotnet

…ng the spanId and traceId into the log context as strings.
…fiers being emitted as strings.

This also adds an AlphabeticalOrderer for XUnit, which is used in `NLogLogProviderTests`. While this is really just a workaround, this solves a small issue with running two test cases consecutively where one uses the `DD_LOGS_INJECTION=true` and uses `DD_LOGS_INJECTION=false`.
@zacharycmontoya zacharycmontoya requested a review from a team as a code owner October 15, 2019 22:49
@zacharycmontoya zacharycmontoya self-assigned this Oct 15, 2019
Copy link
Contributor

@bobuva bobuva 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. Thanks for the walkthrough earlier.

@zacharycmontoya zacharycmontoya added status:do-not-merge Work is done. Can review, but do not merge yet. and removed status:do-not-merge Work is done. Can review, but do not merge yet. labels Oct 16, 2019
@zacharycmontoya zacharycmontoya added this to the 1.8.0 milestone Oct 17, 2019
@zacharycmontoya zacharycmontoya merged commit 3e49bf2 into master Oct 17, 2019
@zacharycmontoya zacharycmontoya deleted the zach/fixes/correlation-identifier-string branch October 17, 2019 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants