Skip to content

Rename and move PeerServiceResolver and friends#16071

Merged
trask merged 3 commits intoopen-telemetry:mainfrom
trask:peer-service-renaming
Feb 10, 2026
Merged

Rename and move PeerServiceResolver and friends#16071
trask merged 3 commits intoopen-telemetry:mainfrom
trask:peer-service-renaming

Conversation

@trask
Copy link
Copy Markdown
Member

@trask trask commented Jan 30, 2026

Related to #16047

This PR renames and refactors the peer service classes to align with the new service.peer.name and service.peer.namespace semantic convention attributes (renamed from peer.service).

Goals

  1. Align naming with semconv - rename from PeerService* to ServicePeer* to match the new attribute names
  2. Reduce public API surface area - the new extractors take OpenTelemetry directly, hiding the resolver implementation as an internal detail
  3. Add service.peer.namespace support - the new resolver supports both service_name and service_namespace configuration

What changed

New classes created

  • service.peer.ServicePeerAttributesExtractor
  • http.HttpClientServicePeerAttributesExtractor

Old classes deprecated

The old classes are deprecated and delegate to the new implementations:

  • net.PeerServiceResolver - deprecated, delegates to ServicePeerResolver
  • net.PeerServiceResolverImpl - deleted (logic moved to ServicePeerResolver)
  • net.PeerServiceAttributesExtractor - deprecated, delegates to ServicePeerAttributesExtractor
  • http.HttpClientPeerServiceAttributesExtractor - deprecated, delegates to HttpClientServicePeerAttributesExtractor

Configuration changes

The declarative config path changed:

  • Old: general.peer.service_mapping
  • New: java.common.service_peer_mapping

(see open-telemetry/opentelemetry-configuration#526 for why)

The config format now supports both service_name and service_namespace:

service_peer_mapping:
  - peer: host
    service_name: serviceName
    service_namespace: serviceNamespace  # new!

Testing

Didn't introduce new tests, since the old tests cover the new classes via the delegation pattern.

When deprecated classes are removed, tests will be updated to point to the new classes.

@github-actions github-actions Bot added the test native This label can be applied to PRs to trigger them to run native tests label Jan 30, 2026
@trask trask requested a review from Copilot January 30, 2026 17:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR renames classes from PeerService* to ServicePeer* to align with the new service.peer.name semantic convention attribute (renamed from peer.service). The PR follows a clean deprecation strategy where old classes delegate to new implementations, making this a safe refactoring.

Changes:

  • Created new ServicePeerResolver, ServicePeerResolverImpl, and ServicePeerAttributesExtractor classes in the semconv.service package
  • Created new HttpClientServicePeerAttributesExtractor in the semconv.http package
  • Deprecated old PeerService* classes which now extend/delegate to the new classes
  • Updated all usages across instrumentation modules to use the new classes
  • Added test coverage for new classes and updated tests for deprecated classes with proper annotations

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ServicePeerResolver.java New interface for service peer resolution with factory methods
ServicePeerResolverImpl.java Internal implementation of ServicePeerResolver (moved from PeerServiceResolverImpl)
ServicePeerAttributesExtractor.java New attributes extractor for service.peer.name attribute
HttpClientServicePeerAttributesExtractor.java New HTTP client-specific attributes extractor with Host header fallback
PeerServiceResolver.java Deprecated interface now extending ServicePeerResolver
PeerServiceResolverImpl.java Deprecated implementation now delegating to ServicePeerResolverImpl
PeerServiceAttributesExtractor.java Deprecated extractor now delegating to ServicePeerAttributesExtractor
HttpClientPeerServiceAttributesExtractor.java Deprecated HTTP extractor now delegating to new class
CommonConfig.java Updated to use ServicePeerResolver type
DefaultHttpClientInstrumenterBuilder.java Updated parameter names and imports
Various instrumentation modules Updated imports and method calls to use new classes
Test files Added tests for new classes, updated deprecated class tests with @SuppressWarnings
AbstractOtelSpringStarterSmokeTest.java Added clarifying comment about JMX metrics scope

Copy link
Copy Markdown
Member Author

@trask trask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Highlighting the diffs better here since the old file still exists and so GitHub diff isn't helpful.

@trask trask marked this pull request as ready for review January 30, 2026 18:44
@trask trask requested a review from a team as a code owner January 30, 2026 18:44
@trask trask mentioned this pull request Jan 30, 2026
14 tasks
@trask trask force-pushed the peer-service-renaming branch from 2dd6281 to 14cf79f Compare February 7, 2026 00:36
@trask trask marked this pull request as draft February 7, 2026 04:25
@trask trask force-pushed the peer-service-renaming branch 4 times, most recently from 3675ea9 to 7523619 Compare February 7, 2026 20:54
@trask trask force-pushed the peer-service-renaming branch from 7523619 to 1ac5f18 Compare February 7, 2026 22:07
implements DeclarativeConfigProperties {

private static final String GENERAL_PEER_SERVICE_MAPPING = "general.peer.service_mapping";
private static final String JAVA_COMMON_SERVICE_PEER_MAPPING = "java.common.service_peer_mapping";
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of this copied from PeerServiceResolverImpl.java

git diff upstream/main:instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/net/PeerServiceResolverImpl.java instrumentation-api-incubator/src/main/java/io/opentelemetry/
instrumentation/api/incubator/semconv/service/peer/internal/ServicePeerResolver.java

diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/net/PeerServiceResolverImpl.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/service/peer/internal/ServicePeerResolver.java
index cc741640fb..d7d7f5faed 100644
--- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/net/PeerServiceResolverImpl.java
+++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/service/peer/internal/ServicePeerResolver.java
@@ -3,51 +3,125 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.instrumentation.api.incubator.semconv.net;
+package io.opentelemetry.instrumentation.api.incubator.semconv.service.peer.internal;
 
+import static java.util.Collections.emptyList;
 import static java.util.Comparator.comparing;
 import static java.util.Comparator.naturalOrder;
 import static java.util.Comparator.nullsFirst;
 
 import com.google.auto.value.AutoValue;
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.instrumentation.api.incubator.config.internal.DeclarativeConfigUtil;
 import io.opentelemetry.instrumentation.api.incubator.semconv.net.internal.UrlParser;
+import io.opentelemetry.instrumentation.api.internal.SemconvStability;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.function.BiConsumer;
 import java.util.function.Supplier;
 import javax.annotation.Nullable;
 
-class PeerServiceResolverImpl implements PeerServiceResolver {
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change at
+ * any time.
+ */
+public class ServicePeerResolver {
+
+  // copied from PeerIncubatingAttributes
+  private static final AttributeKey<String> PEER_SERVICE = AttributeKey.stringKey("peer.service");
+  // copied from ServiceIncubatingAttributes
+  private static final AttributeKey<String> SERVICE_PEER_NAME =
+      AttributeKey.stringKey("service.peer.name");
+  private static final AttributeKey<String> SERVICE_PEER_NAMESPACE =
+      AttributeKey.stringKey("service.peer.namespace");
 
   private static final Comparator<ServiceMatcher> matcherComparator =
       nullsFirst(
           comparing(ServiceMatcher::getPort, nullsFirst(naturalOrder()))
               .thenComparing(comparing(ServiceMatcher::getPath, nullsFirst(naturalOrder()))));
 
-  private final Map<String, Map<ServiceMatcher, String>> mapping = new HashMap<>();
-
-  PeerServiceResolverImpl(Map<String, String> peerServiceMapping) {
-    peerServiceMapping.forEach(
-        (key, serviceName) -> {
-          String url = "https://" + key;
-          String host = UrlParser.getHost(url);
-          Integer port = UrlParser.getPort(url);
-          String path = UrlParser.getPath(url);
-          Map<ServiceMatcher, String> matchers =
-              mapping.computeIfAbsent(host, x -> new HashMap<>());
-          matchers.putIfAbsent(ServiceMatcher.create(port, path), serviceName);
-        });
+  private final Map<String, Map<ServiceMatcher, ServicePeer>> servicePeerMapping = new HashMap<>();
+
+  public ServicePeerResolver(OpenTelemetry openTelemetry) {
+    DeclarativeConfigUtil.getInstrumentationConfig(openTelemetry, "common")
+        .getStructuredList("service_peer_mapping", emptyList())
+        .forEach(
+            entry -> {
+              String peer = entry.getString("peer");
+              String serviceName = entry.getString("service_name");
+              String serviceNamespace = entry.getString("service_namespace");
+              if (peer != null && (serviceName != null || serviceNamespace != null)) {
+                addMapping(peer, serviceName, serviceNamespace);
+              }
+            });
+  }
+
+  @SuppressWarnings("deprecation") // used by deprecated PeerServiceResolver
+  public ServicePeerResolver(Map<String, String> peerServiceNameMapping) {
+    peerServiceNameMapping.forEach((peer, serviceName) -> addMapping(peer, serviceName, null));
+  }
+
+  private void addMapping(
+      String peer, @Nullable String serviceName, @Nullable String serviceNamespace) {
+    if (serviceName == null && serviceNamespace == null) {
+      return;
+    }
+    ServicePeer info = new ServicePeer(serviceName, serviceNamespace);
+    String url = "https://" + peer;
+    String host = UrlParser.getHost(url);
+    Integer port = UrlParser.getPort(url);
+    String path = UrlParser.getPath(url);
+    Map<ServiceMatcher, ServicePeer> matchers =
+        servicePeerMapping.computeIfAbsent(host, x -> new HashMap<>());
+    matchers.putIfAbsent(ServiceMatcher.create(port, path), info);
   }
 
-  @Override
   public boolean isEmpty() {
-    return mapping.isEmpty();
+    return servicePeerMapping.isEmpty();
+  }
+
+  @SuppressWarnings("deprecation") // old semconv
+  public void resolve(
+      String host,
+      @Nullable Integer port,
+      Supplier<String> pathSupplier,
+      BiConsumer<AttributeKey<String>, String> attributeSetter) {
+    ServicePeer servicePeer = resolveServicePeer(host, port, pathSupplier);
+    if (servicePeer == null) {
+      return;
+    }
+
+    String name = servicePeer.name;
+    if (name != null) {
+      if (SemconvStability.emitOldServicePeerSemconv()) {
+        attributeSetter.accept(PEER_SERVICE, name);
+      }
+      if (SemconvStability.emitStableServicePeerSemconv()) {
+        attributeSetter.accept(SERVICE_PEER_NAME, name);
+      }
+    }
+    if (SemconvStability.emitStableServicePeerSemconv()) {
+      String namespace = servicePeer.namespace;
+      if (namespace != null) {
+        attributeSetter.accept(SERVICE_PEER_NAMESPACE, namespace);
+      }
+    }
+  }
+
+  // TODO: remove this method when deprecated PeerServiceResolver is removed
+  @Nullable
+  public String resolveServiceName(
+      String host, @Nullable Integer port, Supplier<String> pathSupplier) {
+    ServicePeer peer = resolveServicePeer(host, port, pathSupplier);
+    return peer != null ? peer.name : null;
   }
 
-  @Override
   @Nullable
-  public String resolveService(String host, @Nullable Integer port, Supplier<String> pathSupplier) {
-    Map<ServiceMatcher, String> matchers = mapping.get(host);
+  ServicePeer resolveServicePeer(
+      String host, @Nullable Integer port, Supplier<String> pathSupplier) {
+    Map<ServiceMatcher, ServicePeer> matchers = servicePeerMapping.get(host);
     if (matchers == null) {
       return null;
     }
@@ -58,11 +132,31 @@ class PeerServiceResolverImpl implements PeerServiceResolver {
         .orElse(null);
   }
 
+  // TODO: remove this method when deprecated PeerServiceResolver is removed
+  @SuppressWarnings("deprecation") // bridges deprecated PeerServiceResolver
+  public static ServicePeerResolver fromPeerServiceResolver(
+      io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceResolver resolver) {
+    return new ServicePeerResolver(new HashMap<>()) {
+      @Override
+      public boolean isEmpty() {
+        return resolver.isEmpty();
+      }
+
+      @Override
+      @Nullable
+      ServicePeer resolveServicePeer(
+          String host, @Nullable Integer port, Supplier<String> pathSupplier) {
+        String serviceName = resolver.resolveService(host, port, pathSupplier);
+        return serviceName != null ? new ServicePeer(serviceName, null) : null;
+      }
+    };
+  }
+
   @AutoValue
   abstract static class ServiceMatcher {
 
     static ServiceMatcher create(@Nullable Integer port, @Nullable String path) {
-      return new AutoValue_PeerServiceResolverImpl_ServiceMatcher(port, path);
+      return new AutoValue_ServicePeerResolver_ServiceMatcher(port, path);
     }
 
     @Nullable
@@ -92,4 +186,15 @@ class PeerServiceResolverImpl implements PeerServiceResolver {
       return true;
     }
   }
+
+  private static final class ServicePeer {
+
+    @Nullable private final String name;
+    @Nullable private final String namespace;
+
+    ServicePeer(@Nullable String name, @Nullable String namespace) {
+      this.name = name;
+      this.namespace = namespace;
+    }
+  }
 }

@trask trask marked this pull request as ready for review February 8, 2026 21:28
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
* any time.
*/
public class ServicePeerResolver {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could make final

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's currently being subclassed as part of the backcompat story

String peer = entry.getString("peer");
String serviceName = entry.getString("service_name");
String serviceNamespace = entry.getString("service_namespace");
if (peer != null && (serviceName != null || serviceNamespace != null)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering whether we should emit a warning when user has specified invalid configuration. Idk whether there are other places where this could also apply.

return;
}
ServicePeer info = new ServicePeer(serviceName, serviceNamespace);
String url = "https://" + peer;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https:// feels random, is it here only to satisfy the url parser?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, added comment (this is carried over from PeerServiceResolverImpl.java)

@trask trask force-pushed the peer-service-renaming branch from 3d5ea91 to 5dfabd7 Compare February 10, 2026 16:15
@trask trask merged commit 3c4ca65 into open-telemetry:main Feb 10, 2026
87 checks passed
@trask trask deleted the peer-service-renaming branch February 10, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test native This label can be applied to PRs to trigger them to run native tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants