Skip to content

Conversation

@raphw
Copy link
Contributor

@raphw raphw commented Apr 25, 2023

Moves all configuration classes to the tracer module such that plugins can access it without depending on the agent core module.

Configuration that is exclusively used by the plugins, is moved to the core module altogether. Configuration that is used by both plugins and the core is split where plugins can access such configuration via a plugin.

@github-actions github-actions bot added agent-java community Issues and PRs created by the community triage labels Apr 25, 2023
@github-actions
Copy link

👋 @raphw Thanks a lot for your contribution!

It may take some time before we review a PR, so even if you don’t see activity for some time, it does not mean that we have forgotten about it.

Every once in a while we go through a process of prioritization, after which we are focussing on the tasks that were planned for the upcoming milestone. The prioritization status is typically reflected through the PR labels. It could be pending triage, a candidate for a future milestone, or have a target milestone set to it.

@raphw raphw changed the title Refactor configuration Extract configuration from core module to API Apr 25, 2023
@ghost
Copy link

ghost commented Apr 25, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview previewSnapshots

Expand to view the summary

Build stats

  • Start Time: 2023-05-24T12:23:52.490+0000

  • Duration: 14 min 0 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark tests.

  • run jdk compatibility tests : Run the JDK Compatibility tests.

  • run integration tests : Run the Agent Integration tests.

  • run end-to-end tests : Run the APM-ITs.

  • run windows tests : Build & tests on windows.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@JonasKunz JonasKunz self-assigned this Apr 26, 2023

static {
checkClassloader();
configs.put(co.elastic.apm.agent.tracer.configuration.CoreConfiguration.class, CoreConfiguration.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

[nitpick]
Imo it would be cleaner to adjust the contract of Tracer.getConfig to allow a lookup based on supertypes as long as the supertype is not ambiguous. To implement this for ElasticApmTracer, we would adapt the ConfigurationRegistry to index not only based on the concrete class but also on the supertypes.
This way no maintenance of this manual mapping is required while not worsening the performance.

However, no need to do this now. I'll maybe tackle this when I work on the config framework anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about this, but the constraint is set by ConfigurationRegistry which requires its keys to be suptypes of the provider class. I found it to messy to paste inn, but hopefully the entire configuration handling can be streamlined some day.

@cla-checker-service
Copy link

cla-checker-service bot commented May 9, 2023

💚 CLA has been signed

@raphw raphw force-pushed the refactor-configuration branch from 5b021df to 313d795 Compare May 9, 2023 11:20
@raphw
Copy link
Contributor Author

raphw commented May 9, 2023

Sorry, had to force push due to a changed user name for demonstrating an exploit somewhere else. Forgot to reset that change.

@SylvainJuge SylvainJuge self-assigned this May 24, 2023
@SylvainJuge
Copy link
Member

@elasticmachine run elasticsearch-ci/docs

@SylvainJuge SylvainJuge enabled auto-merge (squash) May 24, 2023 15:38
@SylvainJuge SylvainJuge merged commit b336370 into elastic:main May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-java community Issues and PRs created by the community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants