You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "weixiang1862 (via GitHub)" <gi...@apache.org> on 2023/06/21 00:45:30 UTC
[GitHub] [skywalking-java] weixiang1862 opened a new pull request, #560: Add WebSphere Liberty 23.x plugin.(#10887)
weixiang1862 opened a new pull request, #560:
URL: https://github.com/apache/skywalking-java/pull/560
<!--
β οΈ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist π ====
### Fix <bug description or the bug issue number or bug issue link>
- [ ] Add a unit test to verify that the fix works.
- [ ] Explain briefly why the bug exists and how to fix it.
==== π Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist π ==== -->
### Add an agent plugin to support WebSphere Liberty
- [x] Add a test case for the new plugin, refer to [the doc](https://github.com/apache/skywalking-java/blob/main/docs/en/setup/service-agent/java-agent/Plugin-test.md)
- [x] Add a component id in [the component-libraries.yml](https://github.com/apache/skywalking/blob/master/oap-server/server-starter/src/main/resources/component-libraries.yml)
- [ ] Add a logo in [the UI repo](https://github.com/apache/skywalking-rocketbot-ui/tree/master/src/views/components/topology/assets)
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist π ====
### Improve the performance of <class or module or ...>
- [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking-java/blob/main/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
- [ ] The benchmark result.
```text
<Paste the benchmark results here>
```
- [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
==== π Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist π ==== -->
<!-- ==== π Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist π ====
### <Feature description>
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
==== π Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist π ==== -->
- [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes [#10887](https://github.com/apache/skywalking/issues/10887)
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking-java/blob/main/CHANGES.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236751811
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
I feel like `javax` version of codes is not tested through the scenario. @weixiang1862 Could you confirm the status?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236253754
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/CompleteRunnableInterceptor.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.websphereliberty.v23;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.AsyncType;
+
+public class CompleteRunnableInterceptor implements InstanceConstructorInterceptor {
+ @Override
+ public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable {
+ // add special flag for AsyncContext distinguish
+ objInst.setSkyWalkingDynamicField(AsyncType.COMPLETE);
Review Comment:
yes, the `CompleteRunnable` is created in `AsyncContextImpl.complete()` method with AsyncType.COMPLETE attribute and then it will be submit to an executor by `AsyncContextImpl.startWithExecutorThread()`. There are no race condition in the whole process, set and get this attribute is always in one thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236295068
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/CompleteRunnableInterceptor.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.websphereliberty.v23;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.AsyncType;
+
+public class CompleteRunnableInterceptor implements InstanceConstructorInterceptor {
+ @Override
+ public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable {
+ // add special flag for AsyncContext distinguish
+ objInst.setSkyWalkingDynamicField(AsyncType.COMPLETE);
Review Comment:
IBM open source liberty code but not release a maven dependency(I can not find it on maven). `CompleteRunnable` and `DispatchRunnable` is not reachable in agent plugin code. That is why I use this strange way rather than `xxx instanceof CompleteRunnable`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#issuecomment-1605885181
> move mock class to com.ibm.** package and add them to check-style exclude list
I can't find it in the change log. There are the changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng merged pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #560:
URL: https://github.com/apache/skywalking-java/pull/560
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236901936
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/pom.xml:
##########
@@ -0,0 +1,103 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ ~
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>apm-sdk-plugin</artifactId>
+ <groupId>org.apache.skywalking</groupId>
+ <version>8.17.0-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>websphere-liberty-23.x-plugin</artifactId>
+
+ <properties>
+ <shade.websphere.target>com.ibm</shade.websphere.target>
+ <shade.websphere.source>org.apache.skywalking.apm.plugin.webspheresource.com.ibm</shade.websphere.source>
+ <junit.version>4.12</junit.version>
+ <javax-servlet-api.version>3.0.1</javax-servlet-api.version>
+ <jakarta-servlet-api.version>6.0.0</jakarta-servlet-api.version>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <version>${junit.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>jakarta.servlet</groupId>
+ <artifactId>jakarta.servlet-api</artifactId>
+ <version>${jakarta-servlet-api.version}</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>javax.servlet</groupId>
+ <artifactId>javax.servlet-api</artifactId>
+ <version>${javax-servlet-api.version}</version>
+ <scope>provided</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <configuration>
+ <includes>
+ <include>org/apache/skywalking/apm/plugin/websphereliberty/**/*</include>
Review Comment:
```suggestion
<include>org/apache/skywalking/apm/plugin/websphereliberty/v23/**</include>
```
And move the `fake` classes into `org/apache/skywalking/apm/plugin/websphereliberty/mock`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#issuecomment-1605886271
> > move mock class to com.ibm.** package and add them to check-style exclude list
>
> I can't find it in the change log. There are the changes?
Yes, mock classes are currently in `com.ibm.**` packages and I add these mock classes to maven-checkstyle-plugin exclude list
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236297657
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/JavaxFilterManagerInvokeInterceptor.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.websphereliberty.v23;
+
+import java.lang.reflect.Method;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+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;
+
+public class JavaxFilterManagerInvokeInterceptor implements InstanceMethodsAroundInterceptor {
+
+ @Override
+ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ MethodInterceptResult result) throws Throwable {
+ HttpServletRequest request = (HttpServletRequest) allArguments[0];
+ ContextCarrier contextCarrier = new ContextCarrier();
+
+ CarrierItem next = contextCarrier.items();
+ while (next.hasNext()) {
+ next = next.next();
+ next.setHeadValue(request.getHeader(next.getHeadKey()));
+ }
+
+ String operationName = String.join(":", request.getMethod(), request.getRequestURI());
+ AbstractSpan span = ContextManager.createEntrySpan(operationName, contextCarrier);
+
+ String url = request.getRequestURL().toString();
+ Tags.URL.set(span, url);
+ Tags.HTTP.METHOD.set(span, request.getMethod());
+
+ span.setComponent(ComponentsDefine.WEBSPHERE);
+
+ SpanLayer.asHttp(span);
+ }
+
+ @Override
+ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ Object ret) throws Throwable {
+ AbstractSpan span = ContextManager.activeSpan();
+
+ int statusCode = ((HttpServletResponse) allArguments[1]).getStatus();
+ Tags.HTTP_RESPONSE_STATUS_CODE.set(span, statusCode);
+
+ if (statusCode >= 400) {
+ span.errorOccurred();
+ }
+
+ ContextManager.stopSpan();
+ return ret;
+ }
+
+ @Override
+ public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
+ Class<?>[] argumentsTypes, Throwable t) {
+ if (ContextManager.isActive()) {
Review Comment:
You are rignt, This check is no necessary. I will remove it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#issuecomment-1600444129
> com.ibm should be in codes, and you should exclude those(com.ibm.*) when package. Could you explain your way?
I use maven-jar-plugin, include codes except fackclass
```
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<configuration>
<includes>
<include>org/apache/skywalking/apm/plugin/websphereliberty/**/*</include>
<include>META-INF/**/*</include>
<include>skywalking-plugin.def</include>
</includes>
</configuration>
</plugin>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236421607
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/CompleteRunnableInterceptor.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.websphereliberty.v23;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.AsyncType;
+
+public class CompleteRunnableInterceptor implements InstanceConstructorInterceptor {
+ @Override
+ public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable {
+ // add special flag for AsyncContext distinguish
+ objInst.setSkyWalkingDynamicField(AsyncType.COMPLETE);
Review Comment:
You could write classes(fake claases) in weblogic namespace and exclude them in the package stage, then, your codes could work without instanceof and have higher perf.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#issuecomment-1600437577
> I am a little confused, why shade?
Checkstyle requires sourcecode package name must start with `org.apache.skywalking.apm.plugin`, so i put websphere fake class under `org.apache.skywalking.apm.plugin.webspheresource.com.ibm.ws.webcontainer.async`.
If not shade, websphere runtime will throw java.lang.NoClassDefFoundError for `org.apache.skywalking.apm.plugin.webspheresource.com.ibm.ws.webcontainer.async.CompleteRunnable.class` missing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236696029
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/AsyncContextInterceptor.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.websphereliberty.v23;
+
+import java.lang.reflect.Method;
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+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.plugin.websphereliberty.v23.async.AsyncType;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.RunnableWrapper;
+
+public class AsyncContextInterceptor implements InstanceMethodsAroundInterceptor {
+
+ @Override
+ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ MethodInterceptResult result) throws Throwable {
+ Runnable runnable = (Runnable) allArguments[0];
+
+ if (runnable instanceof RunnableWrapper) {
Review Comment:
About this `instanceof`, is this executed more than once?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236796956
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
Notice, that(new container) should be another new PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236238727
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/CompleteRunnableInterceptor.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.websphereliberty.v23;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.AsyncType;
+
+public class CompleteRunnableInterceptor implements InstanceConstructorInterceptor {
+ @Override
+ public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable {
+ // add special flag for AsyncContext distinguish
+ objInst.setSkyWalkingDynamicField(AsyncType.COMPLETE);
Review Comment:
This is a class level attribute. Is this class always staying in asyc completed? No race condition?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236615808
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/CompleteRunnableInterceptor.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.websphereliberty.v23;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.AsyncType;
+
+public class CompleteRunnableInterceptor implements InstanceConstructorInterceptor {
+ @Override
+ public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable {
+ // add special flag for AsyncContext distinguish
+ objInst.setSkyWalkingDynamicField(AsyncType.COMPLETE);
Review Comment:
I put fake class file under `org.apache.skywalking.apm.plugin.webspheresource.com.ibm.ws.webcontainer.async`.
`maven-jar-plugin` exclude it from final jar and `maven-shade-plugin` relocation the packagename to `com.ibm.ws.webcontainer.async`.
Please help me review the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236905305
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
@wu-sheng @xu1009 OK, I will do the separate jobs these days.
By the way, Happy Dragon Boat Festival!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236906500
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
You too. Have a good vacation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236989758
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/CompleteRunnableInterceptor.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.websphereliberty.v23;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.AsyncType;
+
+public class CompleteRunnableInterceptor implements InstanceConstructorInterceptor {
+ @Override
+ public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable {
+ // add special flag for AsyncContext distinguish
+ objInst.setSkyWalkingDynamicField(AsyncType.COMPLETE);
Review Comment:
Please try this, list all your `com.ibm.ws.webcontainer.async.*`(all explicit class names) in here, https://github.com/apache/skywalking-java/blob/b72e984289b154c4d7d9714e826ce03482fbf5b3/pom.xml#L411
I hope this could work and avoid shade.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1237165428
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/CompleteRunnableInterceptor.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.websphereliberty.v23;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.AsyncType;
+
+public class CompleteRunnableInterceptor implements InstanceConstructorInterceptor {
+ @Override
+ public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable {
+ // add special flag for AsyncContext distinguish
+ objInst.setSkyWalkingDynamicField(AsyncType.COMPLETE);
Review Comment:
OK, I will try this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236720249
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/AsyncContextInterceptor.java:
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.websphereliberty.v23;
+
+import java.lang.reflect.Method;
+import org.apache.skywalking.apm.agent.core.context.ContextManager;
+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.plugin.websphereliberty.v23.async.AsyncType;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.RunnableWrapper;
+
+public class AsyncContextInterceptor implements InstanceMethodsAroundInterceptor {
+
+ @Override
+ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ MethodInterceptResult result) throws Throwable {
+ Runnable runnable = (Runnable) allArguments[0];
+
+ if (runnable instanceof RunnableWrapper) {
Review Comment:
Yes. In websphere liberty, start runnable created by `AsyncContext.start()` method finally will be submit to an executor by `AsyncContext.startWithExecutorThread` method. So enhance startWithExecutorThread is enough.
But in websphere traditional edition, start method has a check, if `WSAsyncContextImpl.wcConfig.isUseAsyncRunnableWorkManager()==true`, start runnable cant not enter startWithExecutorThread method. So we must enhance start method too.
Both enhance start & startWithExecutorThread will cause AsyncContextInterceptor run twice. So I have a instanceof RunnableWrapper check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236759794
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
> I feel like `javax` version of codes is not tested through the scenario. @weixiang1862 Could you confirm the status?
YesοΌI test this case on websphere traditional 8.5.5 & 9.0.5 in my local. It is a little bit troublesome to create a websphere traditional scenario. Currently there are jvm & tomcat container for scenario test, May be I shoud create a websphere container typeοΌ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236796029
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
If it is EPLv2 as well, we could do that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236431496
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/CompleteRunnableInterceptor.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.websphereliberty.v23;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.AsyncType;
+
+public class CompleteRunnableInterceptor implements InstanceConstructorInterceptor {
+ @Override
+ public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable {
+ // add special flag for AsyncContext distinguish
+ objInst.setSkyWalkingDynamicField(AsyncType.COMPLETE);
Review Comment:
That sounds interesting, I will have a try.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236744349
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
Yes, it is different version of websphere. WebSphere liberty use jakarta and webSphere traditional use javax.
Currnetly WebSphere Liberty 23.x plugin compatible for webSphere liberty and webSphere traditional.
You means I should create another plugin 'websphere-traditional-xx-plugin'οΌ
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#issuecomment-1600423870
I am a little confused, why shade?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236755311
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
> Yes, it is different version of websphere. WebSphere liberty use jakarta and webSphere traditional use javax.
>
> Currnetly WebSphere Liberty 23.x plugin compatible for webSphere liberty and webSphere traditional.
>
> You means I should create another plugin 'websphere-traditional-xx-plugin'οΌ
@weixiang1862 How do you make sure these two kinds of APIs are compiled in one plugin? Just because they have different package names? But how do you test?
I think @xu1009 's point is `witness` class is always recommended to do a version differentiation. Your way works too but a little tricky and implicit.
`witness` + 2 plugins or 2 package names should be better. Also, you should mention this on the plugin docs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1241046583
##########
pom.xml:
##########
@@ -424,7 +424,10 @@
org/apache/skywalking/oap/server/configuration/service/*.java,
**/jmh_generated/*_jmhType*.java,
**/jmh_generated/*_jmhTest.java,
- net/bytebuddy/**
+ net/bytebuddy/**,
+ com/ibm/websphere/servlet/request/IRequest.java,
+ com/ibm/ws/webcontainer/async/CompleteRunnable.java,
+ com/ibm/ws/webcontainer/async/DispatchRunnable.java
Review Comment:
```suggestion
<!-- Exclude compiling auxiliary classes in WebSphere Liberty plugin-->
com/ibm/websphere/servlet/request/IRequest.java,
com/ibm/ws/webcontainer/async/CompleteRunnable.java,
com/ibm/ws/webcontainer/async/DispatchRunnable.java
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236240631
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/JavaxFilterManagerInvokeInterceptor.java:
##########
@@ -0,0 +1,84 @@
+/*
+ * 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.websphereliberty.v23;
+
+import java.lang.reflect.Method;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+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;
+
+public class JavaxFilterManagerInvokeInterceptor implements InstanceMethodsAroundInterceptor {
+
+ @Override
+ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ MethodInterceptResult result) throws Throwable {
+ HttpServletRequest request = (HttpServletRequest) allArguments[0];
+ ContextCarrier contextCarrier = new ContextCarrier();
+
+ CarrierItem next = contextCarrier.items();
+ while (next.hasNext()) {
+ next = next.next();
+ next.setHeadValue(request.getHeader(next.getHeadKey()));
+ }
+
+ String operationName = String.join(":", request.getMethod(), request.getRequestURI());
+ AbstractSpan span = ContextManager.createEntrySpan(operationName, contextCarrier);
+
+ String url = request.getRequestURL().toString();
+ Tags.URL.set(span, url);
+ Tags.HTTP.METHOD.set(span, request.getMethod());
+
+ span.setComponent(ComponentsDefine.WEBSPHERE);
+
+ SpanLayer.asHttp(span);
+ }
+
+ @Override
+ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
+ Object ret) throws Throwable {
+ AbstractSpan span = ContextManager.activeSpan();
+
+ int statusCode = ((HttpServletResponse) allArguments[1]).getStatus();
+ Tags.HTTP_RESPONSE_STATUS_CODE.set(span, statusCode);
+
+ if (statusCode >= 400) {
+ span.errorOccurred();
+ }
+
+ ContextManager.stopSpan();
+ return ret;
+ }
+
+ @Override
+ public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
+ Class<?>[] argumentsTypes, Throwable t) {
+ if (ContextManager.isActive()) {
Review Comment:
Is there a case not `active`? I noticed you added this check in every place. Notice that `isActive` has resource cost too. We only should check when it is expected to be NOT active in some specific known cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] weixiang1862 commented on pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "weixiang1862 (via GitHub)" <gi...@apache.org>.
weixiang1862 commented on PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#issuecomment-1605873572
@wu-sheng @xu1009 I made the following changes in this commit:
1. move mock class to `com.ibm.**` package and add them to check-style exclude list
2. use `com.ibm.ws.webcontainer.WebContainer.handleRequest(IRequest, IResponse)` as interceptor point, it is high level class and hided the detail of real implement(javax or jakarta). So the plugin still keep one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#issuecomment-1600429506
com.ibm should be in codes, and you should exclude those(com.ibm.*) when package.
Could you explain your way?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236447923
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/CompleteRunnableInterceptor.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.websphereliberty.v23;
+
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor;
+import org.apache.skywalking.apm.plugin.websphereliberty.v23.async.AsyncType;
+
+public class CompleteRunnableInterceptor implements InstanceConstructorInterceptor {
+ @Override
+ public void onConstruct(final EnhancedInstance objInst, final Object[] allArguments) throws Throwable {
+ // add special flag for AsyncContext distinguish
+ objInst.setSkyWalkingDynamicField(AsyncType.COMPLETE);
Review Comment:
Notice the license of those fake files. If you only fake some simple methods, it is fine, but ve clear that they are fake classes and not distributed in their class comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236901936
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/pom.xml:
##########
@@ -0,0 +1,103 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ ~
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>apm-sdk-plugin</artifactId>
+ <groupId>org.apache.skywalking</groupId>
+ <version>8.17.0-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>websphere-liberty-23.x-plugin</artifactId>
+
+ <properties>
+ <shade.websphere.target>com.ibm</shade.websphere.target>
+ <shade.websphere.source>org.apache.skywalking.apm.plugin.webspheresource.com.ibm</shade.websphere.source>
+ <junit.version>4.12</junit.version>
+ <javax-servlet-api.version>3.0.1</javax-servlet-api.version>
+ <jakarta-servlet-api.version>6.0.0</jakarta-servlet-api.version>
+ </properties>
+
+ <dependencies>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <version>${junit.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>jakarta.servlet</groupId>
+ <artifactId>jakarta.servlet-api</artifactId>
+ <version>${jakarta-servlet-api.version}</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>javax.servlet</groupId>
+ <artifactId>javax.servlet-api</artifactId>
+ <version>${javax-servlet-api.version}</version>
+ <scope>provided</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <configuration>
+ <includes>
+ <include>org/apache/skywalking/apm/plugin/websphereliberty/**/*</include>
Review Comment:
```suggestion
<include>org/apache/skywalking/apm/plugin/websphereliberty/v23/**</include>
```
And move the `fake` classes into `rg/apache/skywalking/apm/plugin/websphereliberty/mock`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] xu1009 commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "xu1009 (via GitHub)" <gi...@apache.org>.
xu1009 commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236736697
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
com.ibm.ws.webcontainer.filter.WebAppFilterManager.invokeFilters have different arguments in same version websphere?
it seems javax and jakarta are in diiferent version websphere, i think it is better to develop plugins for different version websphere, you can overwrite witnessMethods to identify it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #560: Add WebSphere Liberty 23.x plugin.(#10887)
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #560:
URL: https://github.com/apache/skywalking-java/pull/560#discussion_r1236895543
##########
apm-sniffer/apm-sdk-plugin/websphere-liberty-23.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/websphereliberty/v23/define/JakartaFilterManagerInstrumentation.java:
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.websphereliberty.v23.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 net.bytebuddy.matcher.ElementMatchers.takesArgument;
+import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName;
+
+/**
+ * for websphere liberty WebAppFilterManager enhance
+ */
+public class JakartaFilterManagerInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+
+ private static final String ENHANCE_CLASS = "com.ibm.ws.webcontainer.filter.WebAppFilterManager";
+
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.websphereliberty.v23.JakartaFilterManagerInvokeInterceptor";
+
+ @Override
+ protected ClassMatch enhanceClass() {
+ return byName(ENHANCE_CLASS);
+ }
+
+ @Override
+ public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
+ return null;
+ }
+
+ @Override
+ public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
+ return new InstanceMethodsInterceptPoint[] {
+ new InstanceMethodsInterceptPoint() {
+ @Override
+ public ElementMatcher<MethodDescription> getMethodsMatcher() {
+ return named("invokeFilters")
+ .and(takesArgument(0, named("jakarta.servlet.ServletRequest")))
+ .and(takesArgument(1, named("jakarta.servlet.ServletResponse")));
Review Comment:
@weixiang1862 For this PR, please separate traditional and jakata in two plugins with a parent module `websphere-liberty`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org