Skip to content

Commit

Permalink
up
Browse files Browse the repository at this point in the history 
Signed-off-by: Matthieu MOREL <[email protected]>
Co-Authored-By: Trask Stalnaker <[email protected]>
  • Loading branch information
@mmorel-35 @trask
mmorel-35 and trask committed Sep 2, 2023
1 parent ce1f072 commit 1dd3978
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 103 deletions.
14 changes: 8 additions & 6 deletions ...metry/instrumentation/api/instrumenter/http/HttpClientPeerServiceAttributesExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.net.PeerServiceResolver;
import io.opentelemetry.instrumentation.api.instrumenter.url.UrlParser;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import javax.annotation.Nullable;

Expand All @@ -32,13 +33,14 @@ public final class HttpClientPeerServiceAttributesExtractor<REQUEST, RESPONSE>
}

/**
* Returns a new {@link PeerServiceAttributesExtractor} that will use the passed {@code
* netAttributesExtractor} instance to determine the value of the {@code peer.service} attribute.
* Returns a new {@link HttpClientPeerServiceAttributesExtractor} that will use the passed {@code
* attributesGetter} instance to determine the value of the {@code peer.service} attribute.
*/
public static <REQUEST, RESPONSE> AttributesExtractor<REQUEST, RESPONSE> create(
HttpClientAttributesGetter<REQUEST, RESPONSE> attributesGetter,
PeerServiceResolver peerServiceResolver) {
return new PeerServiceAttributesExtractor<>(attributesGetter, peerServiceResolver);
public static <REQUEST, RESPONSE>
HttpClientPeerServiceAttributesExtractor<REQUEST, RESPONSE> create(
HttpClientAttributesGetter<REQUEST, RESPONSE> attributesGetter,
PeerServiceResolver peerServiceResolver) {
return new HttpClientPeerServiceAttributesExtractor<>(attributesGetter, peerServiceResolver);
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...io/opentelemetry/instrumentation/api/instrumenter/net/PeerServiceAttributesExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public final class PeerServiceAttributesExtractor<REQUEST, RESPONSE>

/**
* Returns a new {@link PeerServiceAttributesExtractor} that will use the passed {@code
* netAttributesExtractor} instance to determine the value of the {@code peer.service} attribute.
* attributesGetter} instance to determine the value of the {@code peer.service} attribute.
*/
public static <REQUEST, RESPONSE> AttributesExtractor<REQUEST, RESPONSE> create(
ServerAttributesGetter<REQUEST, RESPONSE> attributesGetter,
Expand Down
4 changes: 3 additions & 1 deletion .../main/java/io/opentelemetry/instrumentation/api/instrumenter/net/PeerServiceResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
package io.opentelemetry.instrumentation.api.instrumenter.net;

import java.util.Map;
import javax.annotation.Nullable;

public interface PeerServiceResolver {

public boolean isEmpty();

public String resolveService(String host, Integer port, String path);
@Nullable
public String resolveService(String host, @Nullable Integer port, @Nullable String path);

static PeerServiceResolver create(Map<String, String> mapping) {
return new PeerServiceResolverImpl(mapping);
Expand Down
65 changes: 19 additions & 46 deletions ...n/java/io/opentelemetry/instrumentation/api/instrumenter/net/PeerServiceResolverImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
import static java.util.Comparator.naturalOrder;
import static java.util.Comparator.nullsFirst;

import java.io.Serializable;
import com.google.auto.value.AutoValue;
import io.opentelemetry.instrumentation.api.instrumenter.url.UrlParser;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import javax.annotation.Nullable;

class PeerServiceResolverImpl implements PeerServiceResolver {
Expand All @@ -28,18 +28,13 @@ class PeerServiceResolverImpl implements PeerServiceResolver {
PeerServiceResolverImpl(Map<String, String> peerServiceMapping) {
peerServiceMapping.forEach(
(key, serviceName) -> {
String url;
if (key.contains("://")) {
url = key;
} else {
url = "http://" + key;
}
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(new ServiceMatcher(port, path), serviceName);
matchers.putIfAbsent(ServiceMatcher.create(port, path), serviceName);
});
}

Expand All @@ -49,7 +44,8 @@ public boolean isEmpty() {
}

@Override
public String resolveService(String host, Integer port, String path) {
@Nullable
public String resolveService(String host, @Nullable Integer port, @Nullable String path) {
Map<ServiceMatcher, String> matchers = mapping.get(host);
if (matchers == null) {
return null;
Expand All @@ -61,57 +57,34 @@ public String resolveService(String host, Integer port, String path) {
.orElse(null);
}

private static class ServiceMatcher implements Serializable {
@AutoValue
abstract static class ServiceMatcher {

private static final long serialVersionUID = 1L;
@Nullable private final Integer port;
@Nullable private final String path;

ServiceMatcher(Integer port, String path) {
this.port = port;
this.path = path;
static ServiceMatcher create(Integer port, String path) {
return new AutoValue_PeerServiceResolverImpl_ServiceMatcher(port, path);
}

Integer getPort() {
return this.port;
}
@Nullable
abstract Integer getPort();

String getPath() {
return this.path;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (!(o instanceof ServiceMatcher)) {
return false;
}
ServiceMatcher that = (ServiceMatcher) o;
return Objects.equals(port, that.port) && Objects.equals(path, that.path);
}

@Override
public int hashCode() {
return Objects.hash(port, path);
}
@Nullable
abstract String getPath();

public boolean matches(Integer port, String path) {
if (this.port != null) {
if (!this.port.equals(port)) {
if (this.getPort() != null) {
if (!this.getPort().equals(port)) {
return false;
}
}
if (this.path != null && this.path.length() > 0) {
if (this.getPath() != null && this.getPath().length() > 0) {
if (path == null) {
return false;
}
if (!path.startsWith(this.path)) {
if (!path.startsWith(this.getPath())) {
return false;
}
if (port != null) {
return port.equals(this.port);
return port.equals(this.getPort());
}
}
return true;
Expand Down
55 changes: 17 additions & 38 deletions ...ation/api/instrumenter/net/UrlParser.java → ...ation/api/instrumenter/url/UrlParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.instrumenter.net;
package io.opentelemetry.instrumentation.api.instrumenter.url;

import java.util.function.Predicate;
import javax.annotation.Nullable;

class UrlParser {
public class UrlParser {

@Nullable
static String getHost(String url) {
public static String getHost(String url) {

int startIndex = getHostStartIndex(url);
if (startIndex == -1) {
Expand All @@ -26,7 +27,7 @@ static String getHost(String url) {
}

@Nullable
static Integer getPort(String url) {
public static Integer getPort(String url) {

int hostStartIndex = getHostStartIndex(url);
if (hostStartIndex == -1) {
Expand All @@ -53,7 +54,7 @@ static Integer getPort(String url) {
}

@Nullable
static String getPath(String url) {
public static String getPath(String url) {

int hostStartIndex = getHostStartIndex(url);
if (hostStartIndex == -1) {
Expand All @@ -65,23 +66,11 @@ static String getPath(String url) {
return null;
}

if (hostEndIndexExclusive < url.length() && url.charAt(hostEndIndexExclusive) != ':') {
return null;
}

int portStartIndex = hostEndIndexExclusive + 1;

int portEndIndexExclusive = getPortEndIndexExclusive(url, portStartIndex);
if (portEndIndexExclusive == portStartIndex) {
int pathStartIndex = url.indexOf('/', hostEndIndexExclusive);
if (pathStartIndex == -1) {
return null;
}

if (portEndIndexExclusive < url.length() && url.charAt(portEndIndexExclusive) != '/') {
return null;
}

int pathStartIndex = portEndIndexExclusive;

int pathEndIndexExclusive = getPathEndIndexExclusive(url, pathStartIndex);
if (pathEndIndexExclusive == pathStartIndex) {
return null;
Expand Down Expand Up @@ -113,39 +102,29 @@ private static int getHostEndIndexExclusive(String url, int startIndex) {
// look for the end of the host:
// ':' ==> start of port, or
// '/', '?', '#' ==> start of path
int index;
int len = url.length();
for (index = startIndex; index < len; index++) {
char c = url.charAt(index);
if (c == ':' || c == '/' || c == '?' || c == '#') {
break;
}
}
return index;
return getEndIndexExclusive(
url, startIndex, c -> (c == ':' || c == '/' || c == '?' || c == '#'));
}

private static int getPortEndIndexExclusive(String url, int startIndex) {
// look for the end of the port:
// '/', '?', '#' ==> start of path
int index;
int len = url.length();
for (index = startIndex; index < len; index++) {
char c = url.charAt(index);
if (c == '/' || c == '?' || c == '#') {
break;
}
}
return index;
return getEndIndexExclusive(url, startIndex, c -> (c == '/' || c == '?' || c == '#'));
}

private static int getPathEndIndexExclusive(String url, int startIndex) {
// look for the end of the path:
// '?', '#' ==> end of path
return getEndIndexExclusive(url, startIndex, c -> (c == '?' || c == '#'));
}

private static int getEndIndexExclusive(
String url, int startIndex, Predicate<Character> predicate) {
int index;
int len = url.length();
for (index = startIndex; index < len; index++) {
char c = url.charAt(index);
if (c == '?' || c == '#') {
if (predicate.test(c)) {
break;
}
}
Expand Down
25 changes: 15 additions & 10 deletions ...y/instrumentation/api/instrumenter/http/HttpClientPeerServiceAttributesExtractorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.instrumenter.net;
package io.opentelemetry.instrumentation.api.instrumenter.http;

import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
import static java.util.Collections.singletonMap;
Expand All @@ -17,6 +17,7 @@
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.net.PeerServiceResolver;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.HashMap;
import java.util.Map;
Expand All @@ -27,7 +28,7 @@

@ExtendWith(MockitoExtension.class)
class HttpClientPeerServiceAttributesExtractorTest {
@Mock HttpClientAttributesGetter<String, String> netAttributesExtractor;
@Mock HttpClientAttributesGetter<String, String> httpAttributesExtractor;

@Test
void shouldNotSetAnyValueIfNetExtractorReturnsNulls() {
Expand All @@ -36,7 +37,8 @@ void shouldNotSetAnyValueIfNetExtractorReturnsNulls() {
PeerServiceResolver.create(singletonMap("1.2.3.4", "myService"));

HttpClientPeerServiceAttributesExtractor<String, String> underTest =
new PeerServiceAttributesExtractor<>(netAttributesExtractor, peerServiceResolver);
new HttpClientPeerServiceAttributesExtractor<>(
httpAttributesExtractor, peerServiceResolver);

Context context = Context.root();

Expand All @@ -56,9 +58,10 @@ void shouldNotSetAnyValueIfPeerNameDoesNotMatch() {
PeerServiceResolver.create(singletonMap("example.com", "myService"));

HttpClientPeerServiceAttributesExtractor<String, String> underTest =
new PeerServiceAttributesExtractor<>(netAttributesExtractor, peerServiceResolver);
new HttpClientPeerServiceAttributesExtractor<>(
httpAttributesExtractor, peerServiceResolver);

when(netAttributesExtractor.getServerAddress(any())).thenReturn("example2.com");
when(httpAttributesExtractor.getServerAddress(any())).thenReturn("example2.com");

Context context = Context.root();

Expand All @@ -83,9 +86,10 @@ void shouldSetPeerNameIfItMatches() {
PeerServiceResolver peerServiceResolver = PeerServiceResolver.create(peerServiceMapping);

HttpClientPeerServiceAttributesExtractor<String, String> underTest =
new PeerServiceAttributesExtractor<>(netAttributesExtractor, peerServiceResolver);
new HttpClientPeerServiceAttributesExtractor<>(
httpAttributesExtractor, peerServiceResolver);

when(netAttributesExtractor.getServerAddress(any())).thenReturn("example.com");
when(httpAttributesExtractor.getServerAddress(any())).thenReturn("example.com");

Context context = Context.root();

Expand All @@ -99,7 +103,7 @@ void shouldSetPeerNameIfItMatches() {
assertThat(startAttributes.build()).isEmpty();
assertThat(endAttributes.build())
.containsOnly(entry(SemanticAttributes.PEER_SERVICE, "myService"));
verify(netAttributesExtractor, never()).getServerSocketDomain(any(), any());
verify(httpAttributesExtractor, never()).getServerSocketDomain(any(), any());
}

@Test
Expand All @@ -112,9 +116,10 @@ void shouldSetSockPeerNameIfItMatchesAndNoPeerNameProvided() {
PeerServiceResolver peerServiceResolver = PeerServiceResolver.create(peerServiceMapping);

HttpClientPeerServiceAttributesExtractor<String, String> underTest =
new PeerServiceAttributesExtractor<>(netAttributesExtractor, peerServiceResolver);
new HttpClientPeerServiceAttributesExtractor<>(
httpAttributesExtractor, peerServiceResolver);

when(netAttributesExtractor.getServerSocketDomain(any(), any())).thenReturn("example.com");
when(httpAttributesExtractor.getServerSocketDomain(any(), any())).thenReturn("example.com");

Context context = Context.root();

Expand Down
2 changes: 1 addition & 1 deletion ...n/api/instrumenter/net/UrlParserTest.java → ...n/api/instrumenter/url/UrlParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.api.instrumenter.net;
package io.opentelemetry.instrumentation.api.instrumenter.url;

import static org.assertj.core.api.Assertions.assertThat;

Expand Down

0 comments on commit 1dd3978

Please sign in to comment.