Add opt-in target host and port observation tags

micrometer/README.md

@@ -54,6 +54,49 @@ pass it to the constructor:
 new MicrometerObservationCapability(observationRegistry, new MyFeignObservationConvention());
 ```
 
+#### Emitting the target host and port
+
+By default the observation does **not** carry the target host or port. If you
+want them, opt in with the bundled
+`TargetHostAndPortFeignObservationConvention`:
+
+```java
+GitHub github = Feign.builder()
+                     .addCapability(
+                         new MicrometerObservationCapability(
+                             observationRegistry,
+                             TargetHostAndPortFeignObservationConvention.INSTANCE))
+                     .target(GitHub.class, "https://api.github.com");
+```
+
+This adds the following low-cardinality key values on top of the defaults:
+
+| Key | Value |
+| --- | ----- |
+| `net.peer.host` | Host parsed from the configured `Target` url (e.g. `api.github.com`). |
+| `net.peer.port` | Port parsed from the configured `Target` url — **only when the url declares one explicitly**. |
+
+Port semantics:
+
+* The host and port are read from the configured `Target` url
+  (`requestTemplate().feignTarget().url()`), not the final resolved request url.
+* A port is emitted **only if it is explicitly present** in the url. No default
+  port is inferred — `https://api.github.com` yields `net.peer.host` but no
+  `net.peer.port`, while `https://api.github.com:8443` yields both.
+* The port is only emitted alongside a valid host. A missing, invalid, or
+  urless target (such as an `EmptyTarget`) simply omits both keys and never
+  breaks the observation.
+
+> **Cardinality warning:** `net.peer.host` and `net.peer.port` are emitted as
+> *low-cardinality* key values, so they become metric tags. Only enable this
+> convention when the set of target hosts is small and bounded. Against clients
+> that talk to a large or unbounded set of hosts (for example per-tenant
+> hostnames), this can cause a metric-tag cardinality explosion in your metrics
+> backend.
+
+The `DefaultFeignObservationConvention` and its output are unchanged; this
+behavior is strictly opt-in.
+
 ## Metrics published by `MicrometerCapability`
 
 The capability instruments five stages of a Feign call. Each stage emits a

micrometer/src/main/java/feign/micrometer/TargetHostAndPortFeignObservationConvention.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright © 2012 The Feign Authors (feign@commonhaus.dev)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package feign.micrometer;
+
+import io.micrometer.common.KeyValues;
+import io.micrometer.observation.Observation;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+/**
+ * A {@link DefaultFeignObservationConvention} that additionally emits the target host and, when
+ * explicitly present, the target port derived from the Feign {@code Target} url.
+ *
+ * <p>The host and port are taken from {@code
+ * context.getCarrier().requestTemplate().feignTarget().url()} — the configured target url, not the
+ * final resolved request url. Only a port that is explicitly present in that url is emitted; no
+ * default port (80/443) is inferred and no scheme is added. The port is only emitted alongside a
+ * valid host.
+ *
+ * <p>This convention is strictly opt-in via {@link
+ * MicrometerObservationCapability#MicrometerObservationCapability(io.micrometer.observation.ObservationRegistry,
+ * FeignObservationConvention)}. The {@link DefaultFeignObservationConvention} and its output remain
+ * unchanged.
+ *
+ * <p><strong>Cardinality warning:</strong> {@code net.peer.host} and {@code net.peer.port} are
+ * emitted as <em>low-cardinality</em> key values. This is only appropriate when the set of targets
+ * is small and bounded. Enabling this against clients that talk to a large or unbounded set of
+ * hosts (for example, per-tenant hostnames) can cause a metric-tag cardinality explosion in your
+ * metrics backend. Enable it only when you know the target set is limited.
+ *
+ * @author Henrique (henriquejsza)
+ * @see DefaultFeignObservationConvention
+ * @since 13.15
+ */
+public class TargetHostAndPortFeignObservationConvention extends DefaultFeignObservationConvention {
+
+  /** Singleton instance of this convention. */
+  public static final TargetHostAndPortFeignObservationConvention INSTANCE =
+      new TargetHostAndPortFeignObservationConvention();
+
+  // There is no need to instantiate this class multiple times, but it may be extended,
+  // hence protected visibility.
+  protected TargetHostAndPortFeignObservationConvention() {}
+
+  @Override
+  public boolean supportsContext(Observation.Context context) {
+    return context instanceof FeignContext;
+  }
+
+  @Override
+  public KeyValues getLowCardinalityKeyValues(FeignContext context) {
+    KeyValues keyValues = super.getLowCardinalityKeyValues(context);
+    URI targetUri = targetUri(context);
+    if (targetUri == null) {
+      return keyValues;
+    }
+    String host = targetUri.getHost();
+    if (host == null || host.isEmpty()) {
+      return keyValues;
+    }
+    keyValues =
+        keyValues.and(FeignObservationDocumentation.HttpClientTags.TARGET_HOST.withValue(host));
+    int port = targetUri.getPort();
+    if (port != -1) {
+      keyValues =
+          keyValues.and(
+              FeignObservationDocumentation.HttpClientTags.TARGET_PORT.withValue(
+                  String.valueOf(port)));
+    }
+    return keyValues;
+  }
+
+  /**
+   * Parses the configured target url defensively. Returns {@code null} when the url is unavailable
+   * (for example an {@code EmptyTarget}) or is not a valid {@link URI}, so that a missing or
+   * invalid url never breaks the observation.
+   */
+  private static URI targetUri(FeignContext context) {
+    try {
+      String url = context.getCarrier().requestTemplate().feignTarget().url();
+      if (url == null || url.isEmpty()) {
+        return null;
+      }
+      return new URI(url);
+    } catch (URISyntaxException | UnsupportedOperationException ignored) {
+      return null;
+    }
+  }
+}

micrometer/src/test/java/feign/micrometer/TargetHostAndPortFeignObservationConventionTest.java

@@ -0,0 +1,156 @@
+/*
+ * Copyright © 2012 The Feign Authors (feign@commonhaus.dev)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package feign.micrometer;
+
+import feign.Client;
+import feign.Feign;
+import feign.RequestLine;
+import feign.Response;
+import feign.Retryer;
+import feign.Target.EmptyTarget;
+import feign.Target.HardCodedTarget;
+import io.micrometer.observation.tck.TestObservationRegistry;
+import io.micrometer.observation.tck.TestObservationRegistryAssert;
+import java.net.URI;
+import java.util.Collections;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests for {@link TargetHostAndPortFeignObservationConvention}, verifying:
+ *
+ * <ul>
+ *   <li>the {@link DefaultFeignObservationConvention} (the default) never emits host/port;
+ *   <li>the opt-in convention emits host and port when the target url has an explicit port;
+ *   <li>the opt-in convention emits host but no port when the target url has no explicit port;
+ *   <li>an {@link EmptyTarget} (whose {@code url()} throws) never breaks the observation and simply
+ *       omits host/port.
+ * </ul>
+ *
+ * <p>No real network is used: the underlying {@link Client} is stubbed to return a canned 200
+ * response.
+ */
+class TargetHostAndPortFeignObservationConventionTest {
+
+  private static final String TARGET_HOST =
+      FeignObservationDocumentation.HttpClientTags.TARGET_HOST.asString();
+  private static final String TARGET_PORT =
+      FeignObservationDocumentation.HttpClientTags.TARGET_PORT.asString();
+
+  interface TestClient {
+    @RequestLine("GET /")
+    String get();
+  }
+
+  interface AbsoluteUrlClient {
+    @RequestLine("GET")
+    String get(URI uri);
+  }
+
+  private TestObservationRegistry observationRegistry;
+
+  @BeforeEach
+  void setUp() {
+    this.observationRegistry = TestObservationRegistry.create();
+  }
+
+  private static Client okClient() {
+    return (request, options) ->
+        Response.builder()
+            .status(200)
+            .reason("OK")
+            .request(request)
+            .headers(Collections.emptyMap())
+            .build();
+  }
+
+  @Test
+  void defaultConventionDoesNotEmitHostOrPort() {
+    TestClient feignClient =
+        Feign.builder()
+            .client(okClient())
+            .addCapability(new MicrometerObservationCapability(observationRegistry))
+            .target(new HardCodedTarget<>(TestClient.class, "http://localhost:8080"));
+
+    feignClient.get();
+
+    TestObservationRegistryAssert.assertThat(observationRegistry)
+        .hasSingleObservationThat()
+        .hasBeenStopped()
+        .doesNotHaveLowCardinalityKeyValueWithKey(TARGET_HOST)
+        .doesNotHaveLowCardinalityKeyValueWithKey(TARGET_PORT);
+  }
+
+  @Test
+  void optInConventionEmitsHostAndPortWhenPortIsPresent() {
+    TestClient feignClient =
+        Feign.builder()
+            .client(okClient())
+            .addCapability(
+                new MicrometerObservationCapability(
+                    observationRegistry, TargetHostAndPortFeignObservationConvention.INSTANCE))
+            .target(new HardCodedTarget<>(TestClient.class, "http://example.com:8080"));
+
+    feignClient.get();
+
+    TestObservationRegistryAssert.assertThat(observationRegistry)
+        .hasSingleObservationThat()
+        .hasBeenStopped()
+        .hasLowCardinalityKeyValue(TARGET_HOST, "example.com")
+        .hasLowCardinalityKeyValue(TARGET_PORT, "8080");
+  }
+
+  @Test
+  void optInConventionEmitsHostButNoPortWhenPortIsAbsent() {
+    TestClient feignClient =
+        Feign.builder()
+            .client(okClient())
+            .addCapability(
+                new MicrometerObservationCapability(
+                    observationRegistry, TargetHostAndPortFeignObservationConvention.INSTANCE))
+            .target(new HardCodedTarget<>(TestClient.class, "http://example.com"));
+
+    feignClient.get();
+
+    TestObservationRegistryAssert.assertThat(observationRegistry)
+        .hasSingleObservationThat()
+        .hasBeenStopped()
+        .hasLowCardinalityKeyValue(TARGET_HOST, "example.com")
+        .doesNotHaveLowCardinalityKeyValueWithKey(TARGET_PORT);
+  }
+
+  @Test
+  void optInConventionIsSafeWithEmptyTarget() {
+    AbsoluteUrlClient feignClient =
+        Feign.builder()
+            .client(okClient())
+            .retryer(Retryer.NEVER_RETRY)
+            .addCapability(
+                new MicrometerObservationCapability(
+                    observationRegistry, TargetHostAndPortFeignObservationConvention.INSTANCE))
+            .target(EmptyTarget.create(AbsoluteUrlClient.class));
+
+    // EmptyTarget.url() throws UnsupportedOperationException; the convention must swallow it and
+    // simply omit host/port rather than break the observation.
+    feignClient.get(URI.create("http://absolute.example.com:9000/resource"));
+
+    TestObservationRegistryAssert.assertThat(observationRegistry)
+        .hasSingleObservationThat()
+        .hasBeenStopped()
+        .doesNotHaveLowCardinalityKeyValueWithKey(TARGET_HOST)
+        .doesNotHaveLowCardinalityKeyValueWithKey(TARGET_PORT);
+  }
+}