Add `properties` field to the `BaggageEntry` interface

[chrskrchr]

trafficstars

Is your feature request related to a problem? Please describe.

The opentelemetry-js package currently adheres to the key and value definitions in the W3C baggage spec. However, the W3C spec also defines a structured format for additional baggage metadata on each entry in the form of a property set:

3.2.1.4 property

Additional metadata MAY be appended to values in the form of property set, represented as semi-colon ; delimited list of keys and/or key-value pairs, e.g. ;k1=v1;k2;k3=v3. The semantic of such properties is opaque to this specification. Leading and trailing OWS is allowed but MUST be trimmed when converting the header into a data structure.

Describe the solution you'd like

We'd like for the opentelemetry-js package to implement this part of the spec, similar to the opentelemetry-go package:

https://github.com/open-telemetry/opentelemetry-go/blob/main/baggage/baggage.go#L57-L64

This would allow us to propagate and read/write baggage properties across our entire tech stack.

Describe alternatives you've considered

The opentelemetry-js package does expose an opaque metadata string property on the BaggageEntry interface:

https://open-telemetry.github.io/opentelemetry-js-api/interfaces/baggageentry.html#metadata

We could write our own encoder/decoder that transforms the metadata string according to the W3C baggage property spec. However, this seems like something that would be generally valuable for the opentelemetry-js package to do itself.

Additional context

The otel Baggage API spec only mentions the Name, Value and Metadata parameters:

Name The name for which to set the value, of type string.

Value The value to set, of type string.

OPTIONAL parameters:

Metadata Optional metadata associated with the name-value pair. This should be an opaque wrapper for a string with no semantic meaning. Left opaque to allow for future functionality.

Introducing an additional properties property to the BaggageEntry interface doesn't appear be at odds with the spec above.

[chrskrchr]

@dyladan - thanks for adding the up-for-grabs label. Does that imply that you're in support of this feature request? If so, I can take a stab at the implementation.

I logged this feature request against the opentelemetry-js package, but it looks like it would actually be best suited in the baggageEntryMetadataFromString() function in the opentelemetry-js-api package:

https://github.com/open-telemetry/opentelemetry-js-api/blob/main/src/baggage/utils.ts#L35-L56

I think we'd just need to extend the BaggageEntryMetadata type to include a properties: Record<string, string> field and add the serialization/deserialization logic in the baggageEntryMetadataFromString function. Does that sound accurate?

[vmarchaud]

thanks for adding the up-for-grabs label. Does that imply that you're in support of this feature request? If so, I can take a stab at the implementation.

Yeah generally that means anyone is free to take a shot at the PR. If i recall correctly there was a discussion about what to do with the metadata since there was no behavior spec'ed (https://github.com/open-telemetry/opentelemetry-js-api/blob/main/src/baggage/types.ts#L47) and was settled to stay like this until there was one.

[chrskrchr]

Thanks, @vmarchaud! I'll take a stab at the implementation early this week.

My only question at this point is how to make this change in a backwards compatible way (see the very end of the message for the actual question :smile:). I'm proposing we add a new properties field to the BaggageEntry type defined in the opentelemetry-js-api lib:

https://github.com/open-telemetry/opentelemetry-js-api/blob/main/src/baggage/types.ts#L35-L43

export interface BaggageEntry {
  /** `String` value of the `BaggageEntry`. */
  value: string;

  /**
   * Metadata is an optional string property defined by the W3C baggage specification.
   * It currently has no special meaning defined by the specification.
   */
  metadata?: BaggageEntryMetadata;

  // new field - provides access to metadatapropertes via a type like Record<string, string>
  properties?: BaggageEntryProperties;
}

The field addition above is backwards compatible - no issues there.

However, assuming we want to follow existing patterns, this new properties field will be accompanied by a new baggageEntryPropertiesFromRecord() function that's used to create a baggage entry w/ properties, similar to how the existing baggageEntryMetadataFromString() function is used to create an entry with opaque string metadata. This function will be invoked from the opentelemtry-core package when parsing the baggage header into entries:

https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-core/src/baggage/utils.ts#L43-L59

My question - is there a precedent in the otel JS libs for doing a safe/optional import that attempts to import something (e.g., a function), but doesn't blow up if that thing can't be imported? I believe this will be necessary if we want opentelemetry-core to maintain compatibility with @opentelemetry/api >= 1.0.0, which would allow for someone to use the latest version of the core lib but with a version of @opentelemetry/api that doesn't export the new baggageEntryPropertiesFromRecord function.

[chrskrchr]

My question - is there a precedent in the otel JS libs for doing a safe/optional import that attempts to import something (e.g., a function), but doesn't blow up if that thing can't be imported? I believe this will be necessary if we want opentelemetry-core to maintain compatibility with @opentelemetry/api >= 1.0.0, which would allow for someone to use the latest version of the core lib but with a version of @opentelemetry/api that doesn't export the new baggageEntryPropertiesFromRecord function.

Another option would be to break from the existing baggage metadata pattern and NOT introduce a new baggageEntryPropertiesFromRecord function in the API package.

We could still introduce the new properties field on the BaggageEntry, but leave the initialization of that field entirely up to opentelemetry-core. This might require us to simplify the BaggageEntry.properties field type to a simple Record<string, string> interface that doesn't have a corresponding __TYPE__ field. I need to poke around a bit to see how BaggageEntry.metadata.__TYPE__ is being used today to see if breaking from that pattern would introduce issues.

[vmarchaud]

I believe it would be better to add utils/properties to the BaggageEntryMetadata type, it will only require a minor bump then. Note that since the content of metadata isn't spec'ed your utils need to handle un-parsable content.

[chrskrchr]

I believe it would be better to add utils/properties to the BaggageEntryMetadata type

I'd originally considered adding it to BaggageEntryMetadata and doing the deserialization within baggageEntryMetadataFromString(), but it felt a little odd to do that within the @opentelemetry/api package given that the rest of baggage serialization/deserialization was happening within @opentelemetry/core. But I think you're right that this is the only way to make this change without requiring a peer dependency bump or finding a way to gracefully degrade the functionality in the core package when the expected API functions aren't available.

Note that since the content of metadata isn't spec'ed your utils need to handle un-parsable content.

Agreed. Things might get interesting when it comes to determining whether or not the metadata is truly unparsable (versus a funky looking property string), but we can have that conversation when I submit the PR to @opentelemtry/api.

If we're all in agreement that abstracting this change entirely within BaggageEntryMetadata/baggageEntryMetadataFromString() is the desired approach, then the only change I think we'll need here in @opentelemtry/core is to update the baggage serialization logic to include the opaque metadata.toString() value when serializing a baggage entry:

https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-core/src/baggage/utils.ts#L34-L41

I believe this change can be made independently of the other BaggageEntryMetadata.properties change in the API package.

[chrskrchr]

@legendecas @vmarchaud - can we please keep this issue open? #2766 was only the first part of the fix. I still need to submit a PR to @opentelemetry/api to expose the new BaggageEntryMetadata.properties field as described in this comment:

https://github.com/open-telemetry/opentelemetry-js/issues/2740#issuecomment-1030883926

[github-actions[bot]]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[ChrisScotMartin]

Curious what the status is on this, did you ever submit that PR to @opentelemetry/api ?