eclipse / microprofile-metrics

microprofile-metrics
Apache License 2.0
100 stars 66 forks source link

DefaultMetadata Display Name Builder Inconsistency #557

Open jbee opened 4 years ago

jbee commented 4 years ago

When creating a Metadata object with display name intentionally set to not present as in

Metadata.builder().withName(name).withType(metricType).withOptionalDisplayName(null).build()

and such a Metadata object is then modified using

Metadata.builder(metadata).withType(metricType).build()

This changes the display name from null to the value of name which should be unintended and unexpected.

This is especially problematic as by specification the Metadata has to be identical when doing a get or create lookup - otherwise the lookup is an illegal attempt to create the same metric with differing Metadata. (This is not the case as equals uses getDisplayName() which is the name in both cases`)

The problematic line is in constructor of MetadataBuilder

this.displayName = metadata.getDisplayName();

which for DefaultMetadata is implemented as:

Optional.ofNullable(displayName).orElse(name);

Suggested change is to use same pattern for display name as used for unit and description or to simply do a actual copy of the field reference for display name.

If display name should be initialised to name as default this could be done in connection with withName on the builder in case the display name is not set or empty. But considering that getDisplayName() does already return the name field in case displayName is null there is no need to set a default for displayName field.

jbee commented 4 years ago

If think this issue should be fixed by #558