Skip to content

Commit

Permalink
Do not overwrite attributes in ClientObservationConventionAdapter
Browse files Browse the repository at this point in the history
Prior to this commit, the `ClientObservationConventionAdapter` would
overwrite a request builder attribute. This would happen when the
request is not fully built when the observation starts. At that point,
the tags are built for long task timers, but not for the actual metric.
This effectively overrides the correct value of the URI template in the
builder.

This commit removes this builder update which was invalid in the first
place.

Fixes gh-40330
  • Loading branch information
bclozel committed Apr 12, 2024
1 parent 12e004f commit 24f8015
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2022 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -57,7 +57,7 @@ public boolean supportsContext(Observation.Context context) {
public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext context) {
ClientRequest request = context.getRequest();
if (request == null) {
request = context.getCarrier().attribute(URI_TEMPLATE_ATTRIBUTE, context.getUriTemplate()).build();
request = context.getCarrier().build();
}
Iterable<Tag> tags = this.tagsProvider.tags(request, context.getResponse(), context.getError());
return KeyValues.of(tags, Tag::getKey, Tag::getValue);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -83,6 +83,7 @@ void shouldPushTagsAsLowCardinalityKeyValues() {

@Test
void doesNotFailWithEmptyRequest() {
this.context.setUriTemplate(null);
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).contains(KeyValue.of("status", "200"),
KeyValue.of("outcome", "SUCCESS"), KeyValue.of("uri", "/resource/{name}"),
KeyValue.of("method", "GET"));
Expand Down

0 comments on commit 24f8015

Please sign in to comment.