Rewrite plugin using new API#87
Merged
Merged
Conversation
wu-sheng
requested changes
Feb 22, 2017
| */ | ||
| private static String generateOperationName(Invoker<?> invoker, Invocation invocation) { | ||
| StringBuilder operationName = new StringBuilder(); | ||
| operationName.append(invoker.getUrl().getPath()); |
Member
There was a problem hiding this comment.
Multi invocations about invoker.getUrl(), it may be a performance leak.
| StringBuilder requestURL = new StringBuilder(); | ||
| requestURL.append(invoker.getUrl().getProtocol() + "://"); | ||
| requestURL.append(invoker.getUrl().getHost()); | ||
| requestURL.append(":" + invoker.getUrl().getPort() + "/"); |
Member
There was a problem hiding this comment.
Three invoker.getUrl() invocations. same as above.
| import javax.servlet.http.HttpServletResponse; | ||
|
|
||
| /** | ||
| * {@link TomcatInterceptor} fetch the serialized context data by use {@link HttpServletRequest#getHeader(String)}. |
| HttpServletResponse response = (HttpServletResponse) interceptorContext.allArguments()[1]; | ||
|
|
||
| Span span = ContextManager.INSTANCE.activeSpan(); | ||
| Tags.STATUS_CODE.set(span, response.getStatus()); |
Member
There was a problem hiding this comment.
Miss Tags.ERROR.set, which based on response.status, 200 = false, otherwise true.
wu-sheng
requested changes
Feb 23, 2017
| if (url != null) { | ||
| com.weibo.api.motan.rpc.Request request = (com.weibo.api.motan.rpc.Request) interceptorContext.allArguments()[0]; | ||
| Span span = ContextManager.INSTANCE.createSpan(generateViewPoint(url, request)); | ||
| Tags.COMPONENT.set(span, "Motan"); |
| private static String generateOperationName(URL serviceURI, Request request) { | ||
| StringBuilder viewPoint = new StringBuilder(serviceURI.getPath()); | ||
| viewPoint.append("." + request.getMethodName()); | ||
| viewPoint.append("(" + request.getParamtersDesc() + ")"); |
| Tags.SPAN_LAYER.asDB(span); | ||
| if (interceptorContext.allArguments().length > 0 | ||
| && interceptorContext.allArguments()[0] instanceof String) { | ||
| span.setTag("operation_key", (String) interceptorContext.allArguments()[0]); |
wu-sheng
requested changes
Feb 24, 2017
wu-sheng
left a comment
wu-sheng
left a comment
Member
There was a problem hiding this comment.
Comments are not good enough, before merge.
| * {@link DubboInterceptor#afterMethod(EnhancedClassInstanceContext, InstanceMethodInvokeContext, Object)} be executed after | ||
| * {@link com.alibaba.dubbo.monitor.support.MonitorFilter#invoke(Invoker, Invocation)}, and it will check {@link Result#getException()} if is null. | ||
| * current active span will log the exception and set true to the value of error tag if the {@link Result#getException()} is not null. | ||
| */ |
Member
| /** | ||
| * Active span will log the exception and set current span value of error tag. | ||
| */ | ||
| private void dealException(Throwable throwable) { |
Member
There was a problem hiding this comment.
Log the throwable, which occurs in Dubbo RPC service.
|
|
||
| /** | ||
| * Generate operation name. | ||
| * the operation name should be like this <code>com.a.eye.skywalking.plugin.test.Test.test(String)</code>. |
Member
There was a problem hiding this comment.
Format operation name. e.g. com.a.eye.skywalking.plugin.test.Test.test(String)
| * | ||
| * @return operation name. | ||
| */ | ||
| private static String generateOperationName(URL requestURL, Invocation invocation) { |
| /** | ||
| * Set the serialized context data to the first request param that extend {@link SWBaseBean} of dubbo service. | ||
| * | ||
| * @param contextDataStr serialized context data. |
Member
There was a problem hiding this comment.
@param contextDataStr {@link ContextCarrier.serialize()}
| */ | ||
| private static final String ENHANCE_CLASS = "redis.clients.jedis.JedisCluster"; | ||
| /** | ||
| * Class that intercept all constructors with arg. |
Member
There was a problem hiding this comment.
This comment make no sense. Which methods will be intercepted should be commented on the real class.
| private static final String KEY_NAME_OF_REQUEST_URL = "REQUEST_URL"; | ||
|
|
||
| /** | ||
| * The key name that the serialized context data stored in {@link Request#getAttachments()} |
Member
| */ | ||
| private static final String ATTACHMENT_KEY_OF_CONTEXT_DATA = "contextData"; | ||
| /** | ||
| * Motan component |
Member
There was a problem hiding this comment.
This comment make no sense. It is likely:
The 'componet' tag value of Motan RPC framework.
or
A constant for setting the span component name to indicate that it represents a motan client/server span.
| * | ||
| * @author zhangxin | ||
| */ | ||
| public class ProviderInterceptor implements InstanceConstructorInterceptor, InstanceMethodsAroundInterceptor { |
Member
There was a problem hiding this comment.
You should have a naming policy, whether include the component name or not?
| <dependency> | ||
| <groupId>com.a.eye</groupId> | ||
| <artifactId>skywalking-sniffer-mock</artifactId> | ||
| <version>3.0-2017</version> |
wu-sheng
approved these changes
Feb 25, 2017
wu-sheng
left a comment
wu-sheng
left a comment
Member
There was a problem hiding this comment.
Comments need improve. I will try me best.
3 tasks
lu-xiaoshuang
pushed a commit
to lu-xiaoshuang/skywalking
that referenced
this pull request
Aug 12, 2024
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.