Misleading metric label `client.name` in client observation convention

[juliojgd]

The DefaultClientRequestObservationConvention class defines a value for client.name label entry that will be the target host the request will be sent to or the literal "none".

I think the label key should be "target.host" because the value it may contain refers to the target host and not to any other feature of the client.

spring-framework/spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientRequestObservationConvention.java

Lines 136 to 141 in 15364cf

	protected KeyValue clientName(ClientRequestObservationContext context) {
	if (context.getCarrier() != null && context.getCarrier().getURI().getHost() != null) {
	return KeyValue.of(LowCardinalityKeyNames.CLIENT_NAME, context.getCarrier().getURI().getHost());
	}
	return CLIENT_NAME_NONE;
	}

Maybe this name could be changed in the future to clarify the semantics of the contents of that pair?

Thanks for the time to read this

[DhruvTheDev1]

Hi,

The client.name label can be a bit confusing as it actually represents the target host (the server being called) rather than the client making the request.

While the change is small in code, it is a breaking change for users who rely on the current client.name label in their dashboard and metrics.

To avoid breaking existing user's setup, perhaps adding in a new label constant target.host alongside client.name, deprecate client.name and guiding users to migrate to target.host over time?

This way backward compatibility is maintained while improving clarity and correctness of the metric labels.

[bclozel]

I agree that the "client.name" tag might be confusing. I think this dates back from the first HTTP instrumentation with Micrometer (before this was done in Spring directly). We kept this metadata information to make the migration as smooth as possible and not disrupt existing dashboards.

If we were to evolve the observation conventions, we would probably adopt the stable OTel semantic conventions (see #34829), but this is a really disruptive change. I don't think we will rename that field for obvious reasons. We might offer new conventions to align with OTel, but at this point I'm not sure if this will be shipped with Spring Framework directly or with Micrometer. Don't hesitate to follow micrometer-metrics/micrometer#5794 and comment there if this is of interest for you.

@DhruvTheDev1 I'm not sure we should duplicate labels like this. If I'm not mistaken, the cardinality of metadata key/values is important and is directly tied to the cost of the service storing observability data. If this is really a concern, developers can create their own observation convention that inherits from the Spring Framework one, or contribute an observation filter that will rename this metadata on-the-fly.

I'm closing this issue for now as I don't think we should rename this metadata entry, but feel free to discuss things further.