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

Account negotiation is not disabled with management.metrics.export.wavefront.enabled=false #57

Closed
dmurat opened this issue May 29, 2020 · 5 comments · Fixed by #60
Closed
Assignees
Milestone

Comments

@dmurat
Copy link

dmurat commented May 29, 2020

I think it might be useful to have a property for disabling Wavefront integration altogether.

I tried it and want to use it for development. However, rather than poking with dependencies for deployment in upstream environments, it seems that disabling via simple property might be convenient until a broader agreement in an organization is achieved.

Tnx.

@dmurat
Copy link
Author

dmurat commented May 30, 2020

Found a way to do it via spring.autoconfigure.exclude:

spring.autoconfigure.exclude:
  - com.wavefront.spring.actuate.WavefrontEndpointAutoConfiguration
  - com.wavefront.spring.autoconfigure.WavefrontAutoConfiguration
  - org.springframework.boot.actuate.autoconfigure.metrics.export.wavefront.WavefrontMetricsExportAutoConfiguration

Not quite sure if this is enough since AccountManagementEnvironmentPostProcessor still prints out messages about the Wavefront account. Probably is, but the printout is misleading. If possible, maybe it will make sense to add some checks in AccountManagementEnvironmentPostProcessor to back off when WavefrontAutoConfiguration is disabled.

@snicoll
Copy link
Collaborator

snicoll commented May 30, 2020

@dmurat thank you for the report, that's useful information. You can disable Wavefront metrics export using the standard Spring Boot property:

management.metrics.export.wavefront.enabled=false

Not quite sure if this is enough since AccountManagementEnvironmentPostProcessor still prints out messages about the Wavefront account. Probably is, but the printout is misleading.

These are unrelated. The account negotiation happens very early in the lifecycle of the application as it potentially mutates the Environment. It doesn't and can't know about auto-configurations at this stage.

Having said that, disabling itself if the configuration above is defined seems reasonable so we'll do that.

@snicoll snicoll changed the title Property for disabling Wavefront integration Account negotiation is not disabled with management.metrics.export.wavefront.enabled=false May 30, 2020
@snicoll snicoll self-assigned this May 30, 2020
@snicoll snicoll added the bug label May 30, 2020
@snicoll snicoll added this to the 2.0.0 milestone May 30, 2020
@dmurat
Copy link
Author

dmurat commented May 31, 2020

If I understand correctly, once this improvement is implemented, complete turn off will look like the following:

management.metrics.export.wavefront.enabled: false
spring.autoconfigure.exclude:
  - com.wavefront.spring.actuate.WavefrontEndpointAutoConfiguration
  - com.wavefront.spring.autoconfigure.WavefrontAutoConfiguration

Is this correct?

@snicoll
Copy link
Collaborator

snicoll commented May 31, 2020

No. You won’t need to exclude auto-configurations. You can actually do this now but the starter will try to negotiate an account anyway as I’ve described above and that’s what this issue will fix.

@dmurat
Copy link
Author

dmurat commented May 31, 2020

No. You won’t need to exclude auto-configurations.

Oh, that's great :-) Thank you.

snicoll added a commit to snicoll/wavefront-spring-boot that referenced this issue Jun 4, 2020
This commit makes sure we don't unnecessarily negotiate an account if
metrics export has been disabled. This is also makes sure that
`wavefront.freemium-account` takes precedence if set explicitly. The
role and description of the property has also been polished as part of
this commit

Closes wavefrontHQgh-57
snicoll added a commit that referenced this issue Jun 4, 2020
Disable account negotiation if metric exports is disabled
@snicoll snicoll added type: bug and removed bug labels Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants