Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Consolidate to use one JAX-RS filter from com.ibm.ws.jaxrs.2.x.monitor for MP Metrics 5.x #28298

Merged
merged 3 commits into from
May 1, 2024

Conversation

Channyboy
Copy link
Contributor

@Channyboy Channyboy commented Apr 29, 2024

fixes #24693
#build

@Channyboy Channyboy marked this pull request as draft April 29, 2024 21:02
@LibbyBot
Copy link

Your personal build request is at https://libh-proxy1.fyre.ibm.com/cognitive/pipelineAnalysis.html?uuid=6957bb05-dadf-451f-91ef-62e91e80e288

Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

Your personal build request is at https://libh-proxy1.fyre.ibm.com/cognitive/pipelineAnalysis.html?uuid=561c3812-6a38-4f42-9d8d-15e1d617f07f

Target locations of links might be accessible only to IBM employees.

@Channyboy Channyboy force-pushed the RESTFilterExperiment branch 2 times, most recently from 02578a0 to be487c7 Compare April 30, 2024 17:36
@OpenLiberty OpenLiberty deleted a comment from LibbyBot Apr 30, 2024
@OpenLiberty OpenLiberty deleted a comment from LibbyBot Apr 30, 2024
@OpenLiberty OpenLiberty deleted a comment from LibbyBot Apr 30, 2024
@OpenLiberty OpenLiberty deleted a comment from LibbyBot Apr 30, 2024
@OpenLiberty OpenLiberty deleted a comment from LibbyBot Apr 30, 2024
@OpenLiberty OpenLiberty deleted a comment from LibbyBot Apr 30, 2024
@Channyboy
Copy link
Contributor Author

#build
#libby

@@ -109,37 +75,4 @@ public void setMonitorMetricsHandler(MonitorMetricsHandler monitorMetricsHandler
monitorMetricsHandler = monitorMetricsHandlerService;
}

@Activate
protected void activate(ComponentContext context, Map<String, Object> properties) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is related to disabling the filter if the "REST" string is not part of the filter attribute for the monitor element in serer.xml. But since we are removing the filter and using a callback mechanism instead, its no longer needed here.

@@ -39,20 +31,6 @@ public class MonitorAppStateListener implements ApplicationStateListener {

static MonitorMetricsHandler monitorMetricsHandler;

private final static String MONITORING_GROUP_FILTER = "filter";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is related to disabling the filter if the "REST" string is not part of the filter attribute for the monitor element in serer.xml. But since we are removing the filter and using a callback mechanism instead, its no longer needed here.

* JaxRsMonitorFilter class for more information.
*
*/
if (appInfo.getClass().getName().endsWith("EARApplicationInfoImpl")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no longer using a filter in this bundle, therefore no longer need to resolve such information.

@Channyboy Channyboy requested a review from fmhwong April 30, 2024 18:00
statsContext.monitorKey.statsMethodName);

/*
* If we have a MP5RestMetricsCallback service set, follow through to create
Copy link
Contributor Author

Choose a reason for hiding this comment

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

creating REST metric (for mp metrics 5.x ( right when we create the MBean)

@LibbyBot
Copy link

Your personal build request is at https://libh-proxy1.fyre.ibm.com/cognitive/pipelineAnalysis.html?uuid=4ff34d68-7039-4fdb-a824-7beb2bf5a8c2

Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_4gdtoAcVEe-9IeXbkm35aA

Target locations of links might be accessible only to IBM employees.

* @param statsKey the appName[/moduleName]/class/method(params...)
* @return the derived appName
*/
private String resolveAppNameFromStatsKey(String statsKey) {
Copy link
Member

Choose a reason for hiding this comment

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

Can an exception be thrown here rather than using FAILURE_STRING?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

new commit to address this

@Channyboy
Copy link
Contributor Author

#build
#libby

@LibbyBot
Copy link

Your personal build request is at https://libh-proxy1.fyre.ibm.com/cognitive/pipelineAnalysis.html?uuid=ae792f31-1429-4a0d-9562-df9553403578

Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_dwnk4AcrEe-9IeXbkm35aA

Target locations of links might be accessible only to IBM employees.

@@ -12,6 +12,7 @@
*******************************************************************************/
package io.openliberty.restfulws.mpmetrics;

import java.nio.charset.MalformedInputException;
Copy link
Member

Choose a reason for hiding this comment

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

Unused import

@@ -29,6 +30,7 @@

import com.ibm.websphere.ras.Tr;
import com.ibm.websphere.ras.TraceComponent;
import com.ibm.ws.ffdc.annotation.FFDCIgnore;
Copy link
Member

Choose a reason for hiding this comment

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

Unused import

@LibbyBot
Copy link

LibbyBot commented May 1, 2024

@Channyboy Channyboy requested a review from fmhwong May 1, 2024 04:49
@Channyboy
Copy link
Contributor Author

#build
#libby

@LibbyBot
Copy link

LibbyBot commented May 1, 2024

Your personal build request is at https://libh-proxy1.fyre.ibm.com/cognitive/pipelineAnalysis.html?uuid=25afb646-3561-4ea9-b3ae-30197eef10c8

Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

LibbyBot commented May 1, 2024

Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/__5TbcAdtEe-9IeXbkm35aA

Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

LibbyBot commented May 1, 2024

Code analysis and actions

DO NOT DELETE THIS COMMENT.
  • 12 product code files were changed.

  • Please describe in a separate comment how you tested your changes.

  • 1 infrastructure code files were changed.

@LibbyBot
Copy link

LibbyBot commented May 1, 2024

The build Channyboy-28298-20240430-1128
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_4gdtoAcVEe-9IeXbkm35aA
completed successfully!

@LibbyBot
Copy link

LibbyBot commented May 1, 2024

The build Channyboy-28298-20240430-1403
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_dwnk4AcrEe-9IeXbkm35aA
completed successfully!

Copy link
Member

@fmhwong fmhwong left a comment

Choose a reason for hiding this comment

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

LGTM

@LibbyBot
Copy link

LibbyBot commented May 1, 2024

@LibbyBot
Copy link

LibbyBot commented May 1, 2024

The build Channyboy-28298-20240430-2200
https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/__5TbcAdtEe-9IeXbkm35aA
completed successfully!

@Channyboy Channyboy marked this pull request as ready for review May 1, 2024 19:31
@Channyboy Channyboy merged commit 0877a4a into OpenLiberty:integration May 1, 2024
3 of 4 checks passed
@Channyboy Channyboy changed the title Rest filter experiment Consolidate MP Metrics JAX-RS/RESTful-WS filters to use only one in com.ibm.ws.jaxrs.2.x.monitor May 1, 2024
@Channyboy Channyboy changed the title Consolidate MP Metrics JAX-RS/RESTful-WS filters to use only one in com.ibm.ws.jaxrs.2.x.monitor Consolidate to use one JAX-RS filter from com.ibm.ws.jaxrs.2.x.monitor for MP Metrics 5.x May 1, 2024
@Channyboy
Copy link
Contributor Author

Reverting due to issue with #28351 since we don't want to introduce the bug into release (and not enough time to incorporate fix)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dual filter approach causes increase in recorded response times under load.
4 participants