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

CAMEL-20361: camel-jbang - Make jolokia pluggable for camel-platform-http-main #12985

Merged
merged 3 commits into from
Feb 18, 2024

Conversation

kulagaIA
Copy link
Member

@kulagaIA kulagaIA commented Feb 3, 2024

Description

camel-jolokia is now a context plugin with @DevConsole annotation.

I was not able to generate file core/camel-jolokia/src/generated/resources/META-INF/services/org/apache/camel/dev-console/jolokia with the camel-package-maven-plugin. This file was added by hands, and it is possible that something is wrong with camel-jolokia maven configuration, or something is wrong with the maven plugin (see https://camel.zulipchat.com/#narrow/stream/364655-camel-core-dev/topic/how.20to.20add.20context.20plugin/near/419592914 ).

Also added jolokia to example repo: apache/camel-examples#133

Tracking

https://issues.apache.org/jira/browse/CAMEL-20361

Copy link
Contributor

github-actions bot commented Feb 3, 2024

🌟 Thank you for your contribution to the Apache Camel project! 🌟

🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run

  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot.

  • You can label PRs using build-all, build-dependents, skip-tests and test-dependents to fine-tune the checks executed by this PR.

  • Build and test logs are available in the Summary page. Only Apache Camel committers have access to the summary.

  • ⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@kulagaIA
Copy link
Member Author

kulagaIA commented Feb 3, 2024

/component-test camel-platform-http-main

Result ✅ The tests passed successfully

@kulagaIA
Copy link
Member Author

kulagaIA commented Feb 3, 2024

/component-test camel-jolokia
EDIT: camel-jolokia is not a component thats why fail

Result ❌ The tests failed please check the logs

Copy link
Contributor

github-actions bot commented Feb 3, 2024

🤖 The Apache Camel test robot will run the tests for you 👍

1 similar comment
Copy link
Contributor

github-actions bot commented Feb 3, 2024

🤖 The Apache Camel test robot will run the tests for you 👍

/**
* Default {@link org.apache.camel.console.DevConsoleRegistry}.
*/
@DevConsole("jolokia")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is wrong. @devconsole is only for camel-console type of consoles.
You need to come up with another annotation such as JdkFactory.

core/pom.xml Outdated Show resolved Hide resolved
@kulagaIA
Copy link
Member Author

kulagaIA commented Feb 3, 2024

/component-test camel-jolokia

Result ✅ The tests passed successfully

Copy link
Contributor

github-actions bot commented Feb 3, 2024

🤖 The Apache Camel test robot will run the tests for you 👍

@@ -56,6 +56,13 @@
<scope>compile</scope>
</dependency>

<dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

camel-api should NOT have any other dependencies, this must be removed

/**
* Factory to abstract the creation of the Jolokia HttpRequestHandler.
*/
public interface JolokiaHttpRequestHandlerFactory extends StaticService {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make a generic factory that just returns Object or / Class T so we can use this for other stuff we can plugin into http-server in the future.

@@ -370,6 +371,7 @@ protected void initPlugins() {
camelContextExtension.lazyAddContextPlugin(AnnotationBasedProcessorFactory.class,
this::createAnnotationBasedProcessorFactory);
camelContextExtension.lazyAddContextPlugin(DumpRoutesStrategy.class, this::createDumpRoutesStrategy);
camelContextExtension.lazyAddContextPlugin(JolokiaHttpRequestHandlerFactory.class, this::createJolokiaHttpRequestHandlerFactory);
Copy link
Contributor

Choose a reason for hiding this comment

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

Jolokia should only be discovered if enabled, eg that jolokiaEnabled option in camel-main

@davsclaus
Copy link
Contributor

jolokia should be option and not in core at all. Instead there should be a more generic factory for platform-http, that can load jolokia and whatelse we add in the future.

It should be platform-http that when it startup (doInit etc) will check if jolokia is configured to be enabled, and then use the factory to find the JAR (if present) and if present then create the factory and does what it need to do. If the JAR is not present then some exception is thrown. We do this elsewhere in Camel.

@davsclaus
Copy link
Contributor

@kulagaIA I wonder if you will get more time for this before start of next week, as that is the deadline where we will cut the next 4.4 LTS release.

@kulagaIA
Copy link
Member Author

kulagaIA commented Feb 7, 2024

@kulagaIA I wonder if you will get more time for this before start of next week, as that is the deadline where we will cut the next 4.4 LTS release.

Inteding to finish before 12.02.24, is it ok?

@davsclaus
Copy link
Contributor

@kulagaIA I wonder if you will get more time for this before start of next week, as that is the deadline where we will cut the next 4.4 LTS release.
Inteding to finish before 12.07.24, is it ok?

Yes

@kulagaIA kulagaIA force-pushed the camel-jbang-make-jolokia-pluggable branch from ca3fea2 to 95323bb Compare February 15, 2024 18:51
@github-actions github-actions bot removed the core label Feb 15, 2024
@kulagaIA
Copy link
Member Author

kulagaIA commented Feb 15, 2024

/component-test camel-platform-http-main camel-platform-http camel-platform-http-jolokia

Result ❌ The tests failed please check the logs

Copy link
Contributor

🤖 The Apache Camel test robot will run the tests for you 👍

@kulagaIA kulagaIA force-pushed the camel-jbang-make-jolokia-pluggable branch from 95323bb to 579fe34 Compare February 15, 2024 19:02
@kulagaIA
Copy link
Member Author

kulagaIA commented Feb 15, 2024

/component-test camel-platform-http-main camel-platform-http camel-platform-http-jolokia

Result ✅ The tests passed successfully

Copy link
Contributor

🤖 The Apache Camel test robot will run the tests for you 👍

@kulagaIA
Copy link
Member Author

Made PlatformHttpPluginRegistry in camel-platform-http to be a generic factory for platform-http, that can load jolokia and whatelse we add in the future.
Also, apologies for messing up with the commit dates, wanted to finish before 4.4 LTS but got sick

var jolokiaContext = serviceManager.start();
requestHandler = new HttpRequestHandler(jolokiaContext);
}

@Override
public void stop() {
public void doStop() {
serviceManager.stop();
Copy link
Contributor

Choose a reason for hiding this comment

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

Add if (serviceManager != null)

components/camel-platform-http-jolokia/pom.xml Outdated Show resolved Hide resolved
@davsclaus
Copy link
Contributor

Sorry we have now updated main to 4.5.0-SNAPSHOT so you may need to update this PR a little bit, thanks

@kulagaIA kulagaIA force-pushed the camel-jbang-make-jolokia-pluggable branch from 9a248bb to 84b7820 Compare February 17, 2024 20:06
…http-main

PlatformHttpPluginRegistry was made to resolve plugins for camel-platform-http. Jolokia is now camel-platform-http plugin
@davsclaus davsclaus merged commit d622cef into apache:main Feb 18, 2024
2 of 3 checks passed
vishal-bihani pushed a commit to vishal-bihani/camel that referenced this pull request Feb 18, 2024
…http-main (apache#12985)

* CAMEL-20361: camel-jbang - Make jolokia pluggable for camel-platform-http-main
PlatformHttpPluginRegistry was made to resolve plugins for camel-platform-http. Jolokia is now camel-platform-http plugin

* fix version and null check

* up version
vishal-bihani pushed a commit to vishal-bihani/camel that referenced this pull request Feb 18, 2024
vishal-bihani pushed a commit to vishal-bihani/camel that referenced this pull request Feb 18, 2024
…http-main (apache#12985)

* CAMEL-20361: camel-jbang - Make jolokia pluggable for camel-platform-http-main
PlatformHttpPluginRegistry was made to resolve plugins for camel-platform-http. Jolokia is now camel-platform-http plugin

* fix version and null check

* up version
vishal-bihani pushed a commit to vishal-bihani/camel that referenced this pull request Feb 18, 2024
@kulagaIA kulagaIA deleted the camel-jbang-make-jolokia-pluggable branch February 21, 2024 17:32
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.

2 participants