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.

Sign up for GitHub

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

Jump to bottom

Baggage not extracted without trace id from the request #50158

Open
ghost opened this issue Aug 17, 2023 · 2 comments · May be fixed by #55341
Open

Baggage not extracted without trace id from the request #50158

ghost opened this issue Aug 17, 2023 · 2 comments · May be fixed by #55341
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Comments

@ghost
Copy link

ghost commented Aug 17, 2023

The Baggage specification states that the baggage can be used regardless of whether Distributed Tracing is used (at the beginning of https://www.w3.org/TR/baggage).
I wonder why the baggage is not taken into account if we the trace id could not be extracted from the request here:

aspnetcore/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs

Lines 411 to 434 in 31a26e0

if (!string.IsNullOrEmpty(requestId))
{
if (!string.IsNullOrEmpty(traceState))
{
activity.TraceStateString = traceState;
}
var baggage = _propagator.ExtractBaggage(headers, static (object? carrier, string fieldName, out string? fieldValue, out IEnumerable<string>? fieldValues) =>
{
fieldValues = default;
var headers = (IHeaderDictionary)carrier!;
fieldValue = headers[fieldName];
});
// AddBaggage adds items at the beginning of the list, so we need to add them in reverse to keep the same order as the client
// By contract, the propagator has already reversed the order of items so we need not reverse it again
// Order could be important if baggage has two items with the same key (that is allowed by the contract)
if (baggage is not null)
{
foreach (var baggageItem in baggage)
{
activity.AddBaggage(baggageItem.Key, baggageItem.Value);
}
}
}

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions area-hosting Includes Hosting and removed area-runtime area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions labels Aug 25, 2023
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@mkArtakMSFT
Copy link
Member

@amcasey this looks like a OpenTelemetry-related, rather than hosting. Should this be moved back to your area for investigation?

@amcasey
Copy link
Member

amcasey commented Mar 6, 2024

Oops, got confused by the path in the description.

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-hosting Includes Hosting labels Mar 6, 2024
@joegoldman2 joegoldman2 linked a pull request Apr 24, 2024 that will close this issue
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Set the baggage even when the trace id is not successfully extracted joegoldman2/aspnetcore
3 participants
@amcasey @wtgodbe @mkArtakMSFT