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

Fix for missing TraceId and SpanId #196

Merged
merged 2 commits into from
Apr 19, 2024
Merged

Fix for missing TraceId and SpanId #196

merged 2 commits into from
Apr 19, 2024

Conversation

EEParker
Copy link
Collaborator

This PR adds a reproduction for #195 to the sample project.

@EEParker
Copy link
Collaborator Author

image
Sample app reproduction and fix success.

Copy link

Code Coverage

Package Line Rate Branch Rate Health
Serilog.Sinks.Splunk.TCP 65% 43%
Serilog.Sinks.Splunk 19% 19%
Serilog.Sinks.Splunk 49% 63%
Summary 42% (334 / 818) 44% (86 / 208)

@EEParker EEParker self-assigned this Apr 19, 2024
@EEParker EEParker added enhancement Semver-Patch Patch Update - non-breaking changes labels Apr 19, 2024
@EEParker EEParker added this to the 4.1 milestone Apr 19, 2024
@EEParker EEParker merged commit a78df28 into dev Apr 19, 2024
4 checks passed
@EEParker EEParker deleted the issue/195-add-testcase branch April 19, 2024 15:56
@EEParker
Copy link
Collaborator Author

Implement changes from serilog/serilog#1955

@EEParker EEParker changed the title Attempt reproduction of #195 Fix for missing TraceId and SpanId Apr 19, 2024
@VictorioBerra
Copy link
Member

@EEParker overwriting the current TraceIdentifier won't break anything right? Not an expert on that but that's for the entire request right? Would that affect any other middleware?

@EEParker
Copy link
Collaborator Author

Are you referring to the sample app? That is to add a fake traceid since it isn't running aspnetcore. The actual sink is just adding the missing fields from the upstream generic json sink.

@VictorioBerra
Copy link
Member

I was looking at this on my phone and didn't see what app the file was in, looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Semver-Patch Patch Update - non-breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants