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

Changelog, metrics manual instrumentation docs, and 1.3.0 pre-release changes #421

Merged
merged 2 commits into from
Aug 23, 2021

Conversation

mateuszrzeszutek
Copy link
Contributor

No description provided.

@mateuszrzeszutek mateuszrzeszutek requested review from a team as code owners August 23, 2021 09:36
@@ -22,13 +22,63 @@ The following dimensions are automatically added to all metrics exported by the
| `process.pid` | The Java process identifier (PID).
| `service` | The value of the `service.name` resource attribute.

## Manual instrumentation
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've noticed that I forgot to document how to manually add metrics, so I added it right now - that's a feature that needs to be described in this release's docs.

make it very easy to accidentally put the agent on the application runtime
classpath, which may cause all sorts of problems and confusion - and the
agent won't work anyway, because it has to be passed in the `-javaagent` JVM
parameter.
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we removed this because we do publish to maven now. Might be nice to swizzle this around to say something like Q: Should I put the agent on my classpath? A: No, the agent should never be on the runtime classpath. Down the road, we could provide some gradle/mvn snippets that show how to source it without including it in the classpath....I dunno.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, will add.

Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

I had a couple small comments, but this looks good. Thanks for taking this on!

@mateuszrzeszutek mateuszrzeszutek merged commit 7e3a6c3 into main Aug 23, 2021
@delete-merged-branch delete-merged-branch bot deleted the pre-release-1.3.0 branch August 23, 2021 17:31
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