Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intercept feign URL without parameters #3854

Merged
merged 21 commits into from Nov 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -20,6 +20,17 @@

import feign.Request;
import feign.Response;
import org.apache.skywalking.apm.agent.core.context.CarrierItem;
import org.apache.skywalking.apm.agent.core.context.ContextCarrier;
import org.apache.skywalking.apm.agent.core.context.ContextManager;
import org.apache.skywalking.apm.agent.core.context.tag.Tags;
import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;

import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
Expand All @@ -30,16 +41,6 @@
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import org.apache.skywalking.apm.agent.core.context.CarrierItem;
import org.apache.skywalking.apm.agent.core.context.ContextCarrier;
import org.apache.skywalking.apm.agent.core.context.ContextManager;
import org.apache.skywalking.apm.agent.core.context.tag.Tags;
import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;

/**
* {@link DefaultHttpClientInterceptor} intercept the default implementation of http calls by the Feign.
Expand All @@ -48,27 +49,33 @@
*/
public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterceptor {

private static final String COMPONENT_NAME = "FeignDefaultHttp";

/**
* Get the {@link feign.Request} from {@link EnhancedInstance}, then create {@link AbstractSpan} and set host, port,
* kind, component, url from {@link feign.Request}. Through the reflection of the way, set the http header of
* context data into {@link feign.Request#headers}.
*
* @param method
* @param method intercept method
* @param result change this result, if you want to truncate the method.
* @throws Throwable
* @throws Throwable NoSuchFieldException or IllegalArgumentException
*/
@Override public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, MethodInterceptResult result) throws Throwable {
Request request = (Request)allArguments[0];

@Override
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, MethodInterceptResult result) throws Throwable {
Request request = (Request) allArguments[0];
URL url = new URL(request.url());
ContextCarrier contextCarrier = new ContextCarrier();
int port = url.getPort() == -1 ? 80 : url.getPort();
String remotePeer = url.getHost() + ":" + port;
String operationName = url.getPath();
if (operationName == null || operationName.length() == 0) {
FeignResolvedURL feignResolvedURL = PathVarInterceptor.URL_CONTEXT.get();
dominicqi marked this conversation as resolved.
Show resolved Hide resolved
if (feignResolvedURL != null) {
try {
operationName = operationName.replace(feignResolvedURL.getUrl(), feignResolvedURL.getOriginUrl());
} finally {
PathVarInterceptor.URL_CONTEXT.remove();
}
}
if (operationName.length() == 0) {
operationName = "/";
Copy link
Member

Choose a reason for hiding this comment

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

If operationName could be null, operationName.replace would already be NPE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the operationName never be null, because URL.getPath() will return empty str。 I will remove this null condition.

Copy link
Member

@arugal arugal Nov 17, 2019

Choose a reason for hiding this comment

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

It seems better this way. right?

        FeignResolvedURL feignResolvedURL = PathVarInterceptor.URL_CONTEXT.get();
        if (feignResolvedURL != null) {
            try {
                 if (operationName.length() > 0) {
                     operationName = operationName.replace(feignResolvedURL.getUrl(), feignResolvedURL.getOriginUrl());
                  }
            } finally {
                PathVarInterceptor.URL_CONTEXT.remove();
            }
        }
        if (operationName.length() == 0) {
            operationName = "/";
        }

Copy link
Member

Choose a reason for hiding this comment

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

No matter different. Change or not seems OK to me.

Copy link
Member

Choose a reason for hiding this comment

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

No matter different.

ha ha, it seems so.

}
AbstractSpan span = ContextManager.createExitSpan(operationName, contextCarrier, remotePeer);
Expand Down Expand Up @@ -100,14 +107,14 @@ public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterc
* Get the status code from {@link Response}, when status code greater than 400, it means there was some errors in
* the server. Finish the {@link AbstractSpan}.
*
* @param method
* @param ret the method's original return value.
* @return
* @throws Throwable
* @param method intercept method
* @param ret the method's original return value.
* @return origin ret
*/
@Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, Object ret) throws Throwable {
Response response = (Response)ret;
@Override
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, Object ret) {
Response response = (Response) ret;
if (response != null) {
int statusCode = response.status();

Expand All @@ -123,8 +130,9 @@ public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterc
return ret;
}

@Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, Throwable t) {
@Override
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, Throwable t) {
AbstractSpan activeSpan = ContextManager.activeSpan();
activeSpan.log(t);
activeSpan.errorOccurred();
Expand Down
@@ -0,0 +1,58 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF 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 org.apache.skywalking.apm.plugin.feign.http.v9;

/**
* class for {@link PathVarInterceptor} intercept feign url resolved params in url .
* @author qiyang
*/
public class FeignResolvedURL {
/**
* url before resolved
*/
private String originUrl;
/**
* url after resolved
*/
private String url;

public FeignResolvedURL(String originUrl) {
this.originUrl = originUrl;
}

public FeignResolvedURL(String originUrl, String url) {
this.originUrl = originUrl;
this.url = url;
}

public String getOriginUrl() {
return originUrl;
}

public void setOriginUrl(String originUrl) {
this.originUrl = originUrl;
}

public String getUrl() {
return url;
}

public void setUrl(String url) {
this.url = url;
}
}
@@ -0,0 +1,70 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF 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 org.apache.skywalking.apm.plugin.feign.http.v9;

import feign.RequestTemplate;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;

import java.lang.reflect.Method;

/**
* {@link PathVarInterceptor} intercept the Feign RequestTemplate args resolve ;
*
* @author qiyang
*/
public class PathVarInterceptor implements InstanceMethodsAroundInterceptor {

static final ThreadLocal<FeignResolvedURL> URL_CONTEXT = new ThreadLocal<FeignResolvedURL>();

/**
* Get the {@link RequestTemplate#url()} before feign.ReflectiveFeign.BuildTemplateByResolvingArgs#resolve(Object[], RequestTemplate, Map)
* put it into the {@link PathVarInterceptor#URL_CONTEXT}
*
* @param method intercept method
* @param result change this result, if you want to truncate the method.
*/
@Override public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
wu-sheng marked this conversation as resolved.
Show resolved Hide resolved
Class<?>[] argumentsTypes, MethodInterceptResult result) {
RequestTemplate template = (RequestTemplate)allArguments[1];
URL_CONTEXT.set(new FeignResolvedURL(template.url()));
}

/**
* Get the resolved {@link RequestTemplate#url()} after feign.ReflectiveFeign.BuildTemplateByResolvingArgs#resolve(Object[], RequestTemplate, Map)
* put it into the {@link PathVarInterceptor#URL_CONTEXT}
* @param method intercept method
* @param ret the method's original return value.
* @return result without change
*/
@Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, Object ret) {
RequestTemplate resolvedTemplate = (RequestTemplate) ret;
URL_CONTEXT.get().setUrl(resolvedTemplate.url());
return ret;
}

@Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, Throwable t) {
if (URL_CONTEXT.get() != null) {
URL_CONTEXT.remove();
}
}
}
@@ -0,0 +1,68 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF 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 org.apache.skywalking.apm.plugin.feign.http.v9.define;

import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.matcher.ElementMatcher;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint;
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine;
import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;

import static net.bytebuddy.matcher.ElementMatchers.named;
import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;

public class PathVarInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {

/**
* Enhance class.
*/
private static final String ENHANCE_CLASS = "feign.ReflectiveFeign$BuildTemplateByResolvingArgs";

/**
* Intercept class.
*/
private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.feign.http.v9.PathVarInterceptor";

@Override protected ClassMatch enhanceClass() {
return byName(ENHANCE_CLASS);
}

@Override public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
return new ConstructorInterceptPoint[0];
}

@Override public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
return new InstanceMethodsInterceptPoint[] {
new InstanceMethodsInterceptPoint() {
@Override public ElementMatcher<MethodDescription> getMethodsMatcher() {
return named("resolve");
}

@Override public String getMethodsInterceptor() {
return INTERCEPT_CLASS;
}

@Override public boolean isOverrideArgs() {
return false;
}
}
};
}
}
Expand Up @@ -14,4 +14,5 @@
# See the License for the specific language governing permissions and
# limitations under the License.

feign-default-http-9.x=org.apache.skywalking.apm.plugin.feign.http.v9.define.DefaultHttpClientInstrumentation
feign-default-http-9.x=org.apache.skywalking.apm.plugin.feign.http.v9.define.DefaultHttpClientInstrumentation
feign-pathvar-9.x=org.apache.skywalking.apm.plugin.feign.http.v9.define.PathVarInstrumentation