Fix OtlpExporterOptionsExtensions.GetHeaders incorrectly throwing for comma in header value

[tobias-tengler]

Fixes #6510

Changes

Updates OtlpExporterOptionsExtensions.GetHeaders to no longer throw for commas inside of header values.
Previously a valid header value like VL-Stream-Fields=service.name,service.environment would've thrown, since the comma would've been incorrectly interpreted as a delimiter to another header pair - now it's properly parsed.

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: tobias-tengler / name: Tobias Tengler (226edef, f0f44e6, 4e1b471)

[matt-hensley]

likely able to avoid the additional string allocations and only use spans for the new parsing logic

[tobias-tengler]

How do you mean? I'm already using Span for the parsing, but there's no way around allocating a string for the key and value at some point.

[matt-hensley]

initial read looked like there were multiple ToString calls. looks reasonable to push those calls as late as possible

[tobias-tengler]

Do you have a concrete idea of how to do so? I'm not even sure if performance improvements in this part of the code matter that much. This isn't even on any hot path right?

[tebeco]

I feel like the headers shouldn't be a string at all TBH.
the day we'll need to send a Base64 value that's being padded with = what will happen ?

If we're sending headerS (multiples), it sounds like this should be a List<KeyValuePair<string, string>> or something similar @matt-hensley .
Just like Resource/Attribute

I'm not mentionning Dictionary<string, string> because i'm not sure it's good or not to deal with dup / de-dup in headers, I don't know the specs enough

Else it will turn into another footgun TBH

[tobias-tengler]

@tebeco I like the idea.
Just want to add that we for example define the headers as an environment variable that we pass to the string option, since we use different exporters for different environments.
If the API would change to just List<KeyValuePair<string, string>> we would have to do this parsing in user-land.

Which could be fine. It would just add churn for everyone currently using the string option.

[tebeco]

the other thing i haven't looked at all is performance
i don't know the impact of switching from string to List

what i remember is in aspnetcore they use a special StringValues value type because generally a header often only have one value

i have no knowledge of the OTEL specifics here

im just adding it here in case it rings a bell for someone

i hope it doesn't explode the scope of that evol'

[tebeco]

@tebeco I like the idea.

Just want to add that we for example define the headers as an environment variable that we pass to the string option, since we use different exporters for different environments.

If the API would change to just List<KeyValuePair<string, string>> we would have to do this parsing in user-land.

Which could be fine. It would just add churn for everyone currently using the string option.

yeah, i feel the pain, dealing with env var and array is generally not fun

unless you split env var to multiple aggregate yourself and then map it
similar to what dotnet config does for array (but that's not pretty)

i don't think there's such thing as escape in env var
and its also OS specific i assume