Skip to content

Commit

Permalink
Merge branch 'main' into fix-span-stacktrace-capture
Browse files Browse the repository at this point in the history
  • Loading branch information
SylvainJuge authored Jan 8, 2024
2 parents 8d43455 + fa6fed1 commit 363858d
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
[float]
===== Bug fixes
* Fixed NPE in ApacheHttpClientApiAdapter#getHostName - {pull}3479[#3479]
* Fix span stack trace when combined with span compression - {pull}3474[#3474]
[[release-notes-1.x]]
=== Java Agent version 1.x
Expand Down
3 changes: 2 additions & 1 deletion apm-agent-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@
<dependency>
<groupId>org.jctools</groupId>
<artifactId>jctools-core</artifactId>
<version>4.0.2</version>
<!-- 4.0.2 is compiled to target java 8 (for no reason I can see), so keep at 4.0.1 and check 4.0.3 https://github.com/JCTools/JCTools/issues/385 -->
<version>4.0.1</version>
</dependency>
<dependency>
<groupId>com.googlecode.concurrentlinkedhashmap</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import co.elastic.apm.agent.tracer.dispatch.TextHeaderGetter;
import co.elastic.apm.agent.tracer.dispatch.TextHeaderSetter;

import javax.annotation.Nullable;
import java.net.URISyntaxException;

public abstract class AbstractApacheHttpClientAdvice {
Expand All @@ -36,12 +37,12 @@ public abstract class AbstractApacheHttpClientAdvice {
TextHeaderGetter<REQUEST>> Object startSpan(final Tracer tracer,
final ApacheHttpClientApiAdapter<REQUEST, WRAPPER, HTTPHOST, RESPONSE> adapter,
final WRAPPER request,
final HTTPHOST httpHost,
@Nullable final HTTPHOST httpHost,
final HeaderAccessor headerAccessor) throws URISyntaxException {
ElasticContext<?> elasticContext = tracer.currentContext();
Span<?> span = null;
if (elasticContext.getSpan() != null) {
span = HttpClientHelper.startHttpClientSpan(elasticContext, adapter.getMethod(request), adapter.getUri(request), adapter.getHostName(httpHost));
span = HttpClientHelper.startHttpClientSpan(elasticContext, adapter.getMethod(request), adapter.getUri(request), adapter.getHostName(httpHost, request));
if (span != null) {
span.activate();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package co.elastic.apm.agent.httpclient.common;


import javax.annotation.Nullable;
import java.net.URI;
import java.net.URISyntaxException;

Expand All @@ -27,7 +28,8 @@ public interface ApacheHttpClientApiAdapter<REQUEST, WRAPPER extends REQUEST, HT

URI getUri(WRAPPER request) throws URISyntaxException;

CharSequence getHostName(HTTPHOST httpHost);
@Nullable
CharSequence getHostName(@Nullable HTTPHOST httpHost, WRAPPER request);

int getResponseCode(RESPONSE response);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public URI getUri(HttpRequestWrapper request) {
}

@Override
public CharSequence getHostName(HttpHost httpHost) {
public CharSequence getHostName(HttpHost httpHost, HttpRequestWrapper request) {
return httpHost.getHostName();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static class ApacheHttpClient5Advice extends AbstractApacheHttpClientAdvi

@Nullable
@Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
public static Object onBeforeExecute(@Advice.Argument(0) HttpHost httpHost,
public static Object onBeforeExecute(@Advice.Argument(0) @Nullable HttpHost httpHost,
@Advice.Argument(1) ClassicHttpRequest request) throws URISyntaxException {
return startSpan(tracer, adapter, request, httpHost, RequestHeaderAccessor.INSTANCE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,25 @@
package co.elastic.apm.agent.httpclient.v5.helper;

import co.elastic.apm.agent.httpclient.common.ApacheHttpClientApiAdapter;
import co.elastic.apm.agent.sdk.logging.Logger;
import co.elastic.apm.agent.sdk.logging.LoggerFactory;
import org.apache.hc.client5.http.CircularRedirectException;
import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
import org.apache.hc.client5.http.routing.RoutingSupport;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpRequest;

import javax.annotation.Nullable;
import java.net.URI;
import java.net.URISyntaxException;

public class ApacheHttpClient5ApiAdapter implements ApacheHttpClientApiAdapter<HttpRequest, ClassicHttpRequest, HttpHost, CloseableHttpResponse> {
private static final ApacheHttpClient5ApiAdapter INSTANCE = new ApacheHttpClient5ApiAdapter();

private static final Logger logger = LoggerFactory.getLogger(ApacheHttpClient5ApiAdapter.class);

private ApacheHttpClient5ApiAdapter() {
}

Expand All @@ -49,8 +56,19 @@ public URI getUri(ClassicHttpRequest request) throws URISyntaxException {
}

@Override
public CharSequence getHostName(HttpHost httpHost) {
return httpHost.getHostName();
public CharSequence getHostName(@Nullable HttpHost httpHost, ClassicHttpRequest request) {
if (httpHost != null) {
return httpHost.getHostName();
}

try {
return RoutingSupport.determineHost(request)
.getHostName();
} catch (HttpException e) {
logger.error("Exception while determining HostName", e);

return null;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you 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 co.elastic.apm.agent.httpclient.v5;


import co.elastic.apm.agent.httpclient.AbstractHttpClientInstrumentationTest;
import org.apache.hc.client5.http.ClientProtocolException;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
import org.apache.hc.client5.http.impl.classic.HttpClients;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.io.HttpClientResponseHandler;
import org.apache.hc.core5.http.io.entity.EntityUtils;
import org.junit.AfterClass;
import org.junit.BeforeClass;

import java.io.IOException;

public class ApacheHttpClientExecuteOpenInstrumentationTest extends AbstractHttpClientInstrumentationTest {

private static CloseableHttpClient client;

@BeforeClass
public static void setUp() {
client = HttpClients.createDefault();
}

@AfterClass
public static void close() throws IOException {
client.close();
}

/**
* RFC 7230: treat presence of userinfo in authority component in request URI as an HTTP protocol violation.
*
* Uses {@link org.apache.hc.core5.http.message.BasicHttpRequest#setUri} to fill {@link org.apache.hc.core5.net.URIAuthority}
*
* Assertions on authority in {@link org.apache.hc.client5.http.impl.classic.ProtocolExec#execute}
*/
@Override
public boolean isTestHttpCallWithUserInfoEnabled() {
return false;
}

@Override
protected void performGet(String path) throws Exception {
HttpClientResponseHandler<String> responseHandler = response -> {
int status = response.getCode();
if (status >= 200 && status < 300) {
HttpEntity entity = response.getEntity();
String res = entity != null ? EntityUtils.toString(entity) : null;
return res;
} else {
throw new ClientProtocolException("Unexpected response status: " + status);
}
};

ClassicHttpResponse response = client.executeOpen(null, new HttpGet(path), null);
responseHandler.handleResponse(response);
}
}

0 comments on commit 363858d

Please sign in to comment.