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

New Relic Java Agent MeterRegistry #1540

Closed
neiljpowell opened this issue Aug 6, 2019 · 4 comments · Fixed by #1647
Closed

New Relic Java Agent MeterRegistry #1540

neiljpowell opened this issue Aug 6, 2019 · 4 comments · Fixed by #1647
Labels
enhancement A general enhancement registry: new relic A New Relic Registry related issue
Milestone

Comments

@neiljpowell
Copy link
Contributor

neiljpowell commented Aug 6, 2019

@shakuzen We are users of the New Relic Java Agent and would like a MeterRegistry that delegates to the New Relic Insights API available on Agent.
We have created our own delegating implementation that is very similar to the existing NewRelicMeterRegistry, but it provides several additional benefits:

  • Avoids the need for separate API key provisioning/management, which is required for the existing implementation (also not supported by the New Relic PCF Tile)
  • Can limit security proxy/firewall connectivity to collection endpoints used by Agent vs. the Insights REST API (single agent/client publishing metrics to a single monitoring system)
  • Agent provides a reservoir/buffer for metrics processing, similar to Agent APM metrics.

Additionally, we have a MeterRegistryCustomizer/MeterFilter that denies metrics that are already collected by the Agent to avoid duplication, which includes; jvm (buffer, classes, memory, threads), process (cpu, files, start, uptime); system (cpu, load) and http.server.requests.

Finally, we have an MetricsExportAutoConfiguration implementation that load our MeterRegistry based on the presence of the Agent. It also sets the Agent's instanceName and common tags for applicationName and instanceIndex for easier metric correlation via Insights NRQL

@checketts
Copy link
Contributor

@neiljpowell Sounds like you have experience working with New Relic. Are you planning on contributing your code to the project?

If you could do it in such a way to enhance the current NewRelicMeterRegistry and allow enabling your new features via config, that would be ideal.

We could discuss making yours the default behavior, as long as there is a way for users to disable that behavior if they were counting on the old way it was working.

@neiljpowell
Copy link
Contributor Author

@checketts Hi. I do plan to contribute. Enhancing the existing NewRelicMeterRegistry should be fairly easy. I had an opportunity to discuss with @shakuzen and he suggested starting with this Issue submission to help gauge interest.

@shakuzen shakuzen added enhancement A general enhancement registry: new relic A New Relic Registry related issue labels Aug 7, 2019
@shakuzen shakuzen added this to the 1.x milestone Aug 7, 2019
@neiljpowell
Copy link
Contributor Author

neiljpowell commented Aug 16, 2019

@checketts @shakuzen The existing NewRelicMeterRegistry has been update since the last version. It's meter processing methods convert to Stream assembling the Json. The previous processed as a map then converted them all to Json before the REST call, which aligned better with the Agent's Insights API, "recordCustomEvent(string eventType, Map attributes). Adding the feature would result in undoing much of it. I'm thinking that keeping them separate may be better. Thoughts?

@shakuzen
Copy link
Member

I think I'd still rather have it in one NewRelicMeterRegistry. It sounds like there will be some significant internal refactoring, but I think we can get it to a state where it is usable by both options, which will be better long-term. Otherwise we'll have two separate MeterRegistry implementations that need to be maintained and understood by users which to choose. It would also be the first module to have two MeterRegistry implementations in it. If it helps move things along, you can make it separately, open a pull request, and then we can figure out together how to combine them.

@shakuzen shakuzen modified the milestones: 1.x, 1.4.0 Oct 31, 2019
shakuzen pushed a commit that referenced this issue Mar 12, 2020
Introduces the concept of a ClientProvider for the NewRelicMeterRegistry. This allows for different implementations of publishing metrics to New Relic, including the previous HTTP implementation as well as a new one based on the New Relic Java agent for users of that.

Resolves #1540
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement registry: new relic A New Relic Registry related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants