-
Couldn't load subscription status.
- Fork 2
Add logging and metrics for Go, Java and PHP #325
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
Conversation
17edad5 to
bc89d89
Compare
| // Log different severity levels | ||
| julLogger.info("Application started - java.util.logging"); | ||
| julLogger.log(Level.WARNING, "This is a warning message - java.util.logging"); | ||
| julLogger.severe("This is an error message - java.util.logging"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool to see the logging adds the trace and span id. We could use that to link to search.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! It's not the only one that does it, I believe. I'll create an issue for it. We'll also have to account for the different ways to send this data that exist, because of course they do.
java/spring-agent-collector/app/src/main/java/io/opentelemetry/example/graal/Controller.java
Outdated
Show resolved
Hide resolved
| $gauge->record(10); | ||
|
|
||
| // Histogram metric for distributions (measurements) | ||
| $histogram = $meter->createHistogram('my_histogram'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The histograms are not being reported for (using a locally build collector using the main branch). I get this message:
appsignal-collector-1 | [2025-08-07T16:53:28 (collector) #1][Debug] Ignoring OpenTelemetry histogram metric: 'my_histogram': Missing min value
appsignal-collector-1 | [2025-08-07T16:53:28 (collector) #1][Debug] Ignoring OpenTelemetry histogram metric: 'my_tagged_histogram': Missing min value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, thank you for spotting this! Another ✨ fun ✨ change to the histogram-to-distributions hack incoming, I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, after implementing that and testing that: turns out what the PHP histogram implementation actually does is send exemplars. The bad news is now I have to implement exemplars support. The good news is that exemplars are exactly what we wish we'd get from OpenTelemetry, rather than a histogram that we need to invent plausible values for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay 👍 Is the PR you linked up-to-date then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That PR is fine and ready to merge IMO (I think it's an overall improvement) but there will be a second PR that actually implements what's necessary in order to support PHP histograms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok! Let's merge it then.
bc89d89 to
d830e41
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving because I trust the setup is good, but the collector is the one that needs to be updated.
Add endpoints that emit logs and metrics, where missing, to some of the Go, Java and PHP test setups.
This PR is a complement to the docs' PR: https://github.com/appsignal/appsignal-docs/pull/2786