Feedback on ExtendedAttributes API

[fandreuz]

Hi @jack-berg, thanks for working on #7123. As discussed below the PR, I'll drop here some feedback I collected while working on open-telemetry/opentelemetry-java-instrumentation#8354. There may be more, I'll update the issue if I think about something else.

InternalExtendedAttributeKeyImpl#toString should contain type information

The current implementation makes inspecting test failures quite confusing:

    Expecting map:
      {string1="1", string2="2"}
    to contain entries:
      [map={string1="1", string2="2"}]
    but could not find the following map entries:
      [map={string1="1", string2="2"}]

I think it would make sense to embed the additional information in the string.

ExtendedAttributesBuilder#put(String, Object)

Sometimes the type information at compile time is erased due to attributes being pushed into a Map<String, Object> (e.g. Log4j's ThreadContext#getThreadContextMap). Thus, we have to write something like this:

    if (value instanceof String) {
      attributes.put(
          (ExtendedAttributeKey<String>) keyProvider.apply(key, ExtendedAttributeType.STRING),
          (String) value);
    } else if (value instanceof Boolean) {
      attributes.put(
          (ExtendedAttributeKey<Boolean>) keyProvider.apply(key, ExtendedAttributeType.BOOLEAN),
          (Boolean) value);
[...]

This same logic might be duplicated in many different places, increasing the likelihood of mistakes. Example here. Is there a better way to do it in the current implementation?

It would be nice to have a public utility to expose InternalExtendedAttributeKeyImpl.create

Sometimes it's useful to use this factory directly, e.g. here. Should I consider it public API? I guess not.

It would be nice to extend the assertion framework for ExtendedAttributes

Writing tests for ExtendedAttributes is possible at the moment, but a bit cumbersome. I guess LogRecordDataAssert will be updated only when the API stabilizes, right?