instrumentation config: return empty object when not set

[zeitlinger]

Fixes open-telemetry/opentelemetry-java#7958

Changes

While working on Java, it has turned out to be more convenient to work with empty config objects rather than null/nil.

• null requires an explicit if, making the code more verbose
• it doesn't matter if the .instrumentation is missing or is empty, which is a different question than "is declarative config used")

• Related issues: add method to retrieve instrumentation configuration by name opentelemetry-java#7927 (comment)
• Related OTEP(s) #
• Links to the prototypes (when adding or changing features): make getInstrumentationConfig non-null opentelemetry-java#7954
• CHANGELOG.md file updated for non-trivial changes
• Spec compliance matrix updated if necessary

[ysolomchenko]

I agree that it doesn’t matter whether .instrumentation is missing or empty.

However, I don’t agree that null requires an explicit if, making the code more verbose.
If the value is null and we check for that once, there is no need to perform further checks.
If it is an empty object, on the other hand, we still need to check whether the objects inside it are null.

Even though this change does not affect functionality, from my perspective I don’t see why it is needed. There is no clear benefit in having an empty object when we can use null instead.

[jkwatson]

Even though this change does not affect functionality, from my perspective I don’t see why it is needed. There is no clear benefit in having an empty object when we can use null instead.

It's a matter of API consistency. If some methods return empty objects and some return null, it's more difficult for consumers of the API to know which is which and when they should check for null and when they shouldn't. I pushed for non-null to match with other methods on the configuration objects. Consistency in an API is important; parallel methods should return the same types. Either all nulls, or all empty objects, but not an arbitrary mix of both.

[trask]

If the value is null and we check for that once

I think there are a couple of assumptions here:

• we remember to check for it
• there's only one place we need to check it (in the Java Instrumentation repo we are currently checking for it 144 times)

(btw, I do agree that some languages make dealing with nulls easier)

If it is an empty object, on the other hand, we still need to check whether the objects inside it are null.

yeah, this is also a big usability issue we ran into when migrating the Java Instrumentation to the declarative configuration API

to avoid this in Java, we added an additional convenience accessor to ConfigProperties which returns nested mappings or empty, so that we can just chain without checking for null at each step, and we only need to check the final scalar value

[zeitlinger]

and we only need to check the final scalar value

this is an example:

@Bean
  static SpringKafkaTelemetry getTelemetry(ObjectProvider<OpenTelemetry> openTelemetryProvider) {
    OpenTelemetry openTelemetry = openTelemetryProvider.getObject();
    return SpringKafkaTelemetry.builder(openTelemetry)
        .setCaptureExperimentalSpanAttributes(
            DeclarativeConfigUtil.getInstrumentationConfig(openTelemetry, "kafka")
                .getBoolean("experimental_span_attributes/development", false))
        .setMessagingReceiveTelemetryEnabled(
            DeclarativeConfigUtil.getInstrumentationConfig(openTelemetry, "common")
                .get("messaging")
                .get("receive_telemetry/development")
                .getBoolean("enabled", false))
        .setCapturedHeaders(
            DeclarativeConfigUtil.getInstrumentationConfig(openTelemetry, "common")
                .get("messaging")
                .getScalarList("capture_headers/development", String.class, emptyList()))
        .build();
  }