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

Allow file-based configuration for Dynatrace v2 exporter #2916

Conversation

pirgeo
Copy link
Contributor

@pirgeo pirgeo commented Dec 13, 2021

This PR introduces support for file-based configuration using the DynatraceFileBasedConfigurationProvider. It also makes sure changes in the file-based configuration are picked up, without having to restart the app to change configuration.

This change adds another layer to the auto-configuration that is already available: Instead of falling back to the local OneAgent directly, the the file at /var/lib/dynatrace/enrichment/endpoint/endpoint.properties will be checked for configuration. The file will typically be provided by the Dynatrace Operator but can also just be added manually if you'd like to test it locally. I will follow up with a PR updating the documentation in micrometer-docs to describe the changes.

@pirgeo
Copy link
Contributor Author

pirgeo commented Feb 15, 2022

Hey @jonatan-ivanov and @shakuzen, it would be great if someone could take a look at this PR please, it has been sitting for a while now.
Do you think it's possible to get this into the 1.9.0 release?
Thank you!

Copy link
Member

@jonatan-ivanov jonatan-ivanov left a comment

Choose a reason for hiding this comment

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

Let's merge this and I'm going to add a polish commit.

@jonatan-ivanov jonatan-ivanov added this to the 1.9.0-M3 milestone Feb 15, 2022
@jonatan-ivanov jonatan-ivanov added enhancement A general enhancement registry: dynatrace A Dynatrace Registry related issue labels Feb 15, 2022
@jonatan-ivanov jonatan-ivanov merged commit 8cedbdf into micrometer-metrics:main Feb 15, 2022
@jonatan-ivanov
Copy link
Member

Thanks @pirgeo!

public class FileBasedConfigurationTestHelper {
// This allows overwriting the file location of the file-based configuration for testing.
public static void forceOverwriteConfig(String filename) {
DynatraceFileBasedConfigurationProvider.getInstance().forceOverwriteConfig(filename);
Copy link
Member

Choose a reason for hiding this comment

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

I've just realized why is this needed.
Is it possible to make it public or release a test dependency where this is available?

jonatan-ivanov added a commit to jonatan-ivanov/micrometer that referenced this pull request Feb 15, 2022
@jonatan-ivanov
Copy link
Member

jonatan-ivanov commented Feb 15, 2022

@pirgeo I think there are some issues with the Dynatrace file poller, it seems it takes a lot of time on macOS for the file poller to detect the changes.

The tests in this PR were failing for me on my mac (OS X 12.1, x86_64): see this report.
I replaced Thread::sleep with awaitility it takes almost 10 seconds on my machine to see the file changes (instead of 10ms that you had). Could you please look into it if anyone else has this issue before?

My changes: jonatan-ivanov@f4c1c42 (they are unrelated since the build was failing for me at the first place).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement registry: dynatrace A Dynatrace Registry related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants