Skip to content

Pyroscope agent support#5303

Merged
deejgregor merged 1 commit intodevelopfrom
deejgregor/pyroscope-agent
Sep 22, 2022
Merged

Pyroscope agent support#5303
deejgregor merged 1 commit intodevelopfrom
deejgregor/pyroscope-agent

Conversation

@deejgregor
Copy link
Copy Markdown
Contributor

@deejgregor deejgregor commented Sep 21, 2022

https://github.com/pyroscope-io/otel-profiling-java

To use:

  1. Install Pyroscope server: https://pyroscope.io/docs/installing-pyroscope-overview/
  2. Set PYROSCOPE_AGENT_ENABLED=1 in opennms.conf
  3. Restart OpenNMS.
  4. Go to http://localhost:4040/

If the Pyroscope server is hosted somewhere else or you want to call the application something other than "OpenNMS", set the PYROSCOPE_SERVER_ADDRESS or PYROSCOPE_APPLICATION_NAME variables appropriately.

You can also set other configuration options mentioned in https://pyroscope.io/docs/java/#configuration however you will also need to export those variables in your opennms.conf.

Future work:

  • Unify existing profiling/debugging/etc. features in the opennms startup script once we have a better idea of what is still used/usable and remove outdated options.
  • Create documentation somewhere on these features.

All Contributors

Contribution Checklist

  • Please make an issue in the OpenNMS issue tracker if there isn't one already.
    Once there is an issue, please:
    1. update the title of this PR to be in the format: ${JIRA-ISSUE-NUMBER}: subject of pull request
    2. update the JIRA link at the bottom of this comment to refer to the real issue number
    3. prefix your commit messages with the issue number, if possible
    4. once you've created this PR, please link to it in a comment in the JIRA issue
      Don't worry if this sounds like a lot, we can help you get things set up properly.
  • If this code is likely to affect the UI, did you name your branch with -smoke in it to trigger smoke tests?
  • If this is a new or updated feature, is there documentation for the new behavior?
  • If this is new code, are there unit and/or integration tests?
  • If this PR targets a foundation-* branch, does it try to avoid changing files in $OPENNMS_HOME/etc/?

What's Next?

A PR should be assigned at least 2 reviewers. If you know that someone would be a good person to review your code, feel free to add them.

If you need help making additions or changes to the documentation related to your changes, please let us know.

In any case, if anything is unclear or you want help getting your PR ready for merge, please don't hesitate to say something in the comments here,
or in the #opennms-development chat channel.

Once reviewer(s) accept the PR and the branch passes continuous integration, the PR is eligible for merge.

At that time, if you have commit access (are an OpenNMS Group employee or a member of the OGP) you are welcome to merge the PR when you're ready.
Otherwise, a reviewer can merge it for you.

Thanks for taking time to contribute!

External References

https://github.com/pyroscope-io/otel-profiling-java

To use:
1. Install Pyroscope server:
    https://pyroscope.io/docs/installing-pyroscope-overview/
2. Set PYROSCOPE_AGENT_ENABLED=1 in opennms.conf
3. Restart OpenNMS.
4. Go to http://localhost:4040/

If the Pyroscope server is hosted somewhere else or you want to call the
application something other than "OpenNMS", set the PYROSCOPE_SERVER_ADDRESS
or PYROSCOPE_APPLICATION_NAME variables appropriately.

You can also set other configuration options mentioned in
https://pyroscope.io/docs/java/#configuration however you will also need to
export those variables in your opennms.conf.

Future work:
- Unify existing profiling/debugging/etc. features in the opennms startup
script once we have a better idea of what is still used/usable and remove
outdated options.
- Create documentation somewhere on these features.
@deejgregor
Copy link
Copy Markdown
Contributor Author

Note: if using Homebrew OpenJDK 11, make sure that you are running 11.0.16.1_1 or later (note the _1 for the package revision on the end) so that you have this fix: Homebrew/homebrew-core#111255

Copy link
Copy Markdown
Contributor

@RangerRick RangerRick left a comment

Choose a reason for hiding this comment

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

not sure how I feel about a new top-level "agent" directory, but this all looks reasonable to me

@deejgregor
Copy link
Copy Markdown
Contributor Author

not sure how I feel about a new top-level "agent" directory, but this all looks reasonable to me

I'm trying to keep it out of 'lib', but aside from that, I don't care about where it goes.

Note: This isn't so much for Pyroscope now, but for the OpenTelemetry agent down the road, which has things like its own version of Guava, etc.. They have a whole class loader setup to keep their bits out of the system class loader: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/contributing/javaagent-structure.md (picture of structure at the very bottom).

@RangerRick
Copy link
Copy Markdown
Contributor

I'm trying to keep it out of 'lib', but aside from that, I don't care about where it goes.

Yeah, I figured.

Note: This isn't so much for Pyroscope now, but for the OpenTelemetry agent down the road, which has things like its own version of Guava, etc.. They have a whole class loader setup to keep their bits out of the system class loader: https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/contributing/javaagent-structure.md (picture of structure at the very bottom).

Alright, if it's gonna become a place for a few different things, that's cool.

@deejgregor deejgregor merged commit 6dfc41e into develop Sep 22, 2022
@deejgregor deejgregor deleted the deejgregor/pyroscope-agent branch September 22, 2022 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants