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

Add logs #158

Merged
merged 3 commits into from
Apr 9, 2024
Merged

Add logs #158

merged 3 commits into from
Apr 9, 2024

Conversation

Emily-Jiang
Copy link
Member

No description provided.

Copy link
Contributor

@donbourne donbourne left a comment

Choose a reason for hiding this comment

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

The main question we need to consider is whether there is any need for the logs API to be exposed to applications. The alternative is that we could just require that runtimes that support MP Telemetry provide a way to route the runtime and app's logs to OpenTelemetry.

spec/src/main/asciidoc/opentelemetry-apis.adoc Outdated Show resolved Hide resolved
@pdudits
Copy link
Contributor

pdudits commented Mar 11, 2024

Like @donbourne noted, this API is not really intended for end users. Rather the runtimes should assure that their application and system logs are exported over appropriate OpenTelemetry channel.

For that we would need:

  • support configuration of log exporters otel.logs.exporter
  • document service loading capability of LogRecordExporter

Then the reasonable requirement would be that application's logs capture reasonable context information and other telemetry attributes -- the feature that could actually be implemented on top of LogRecordBuilder. But there's no standard we can build upon here as each vendor uses different logging API and there's no common one that would be supported in applications e. g. by means of injection.

@Emily-Jiang
Copy link
Member Author

Is JUL the one every runtime supports?

@Emily-Jiang
Copy link
Member Author

I believe I have addressed all of your comments @donbourne @pdudits @brunobat . Please re-review and approve or add more comments. Thanks

Copy link
Contributor

@donbourne donbourne left a comment

Choose a reason for hiding this comment

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

looks good - just one very minor comment.

spec/src/main/asciidoc/configuration.adoc Show resolved Hide resolved
@Emily-Jiang
Copy link
Member Author

@brunobat @pdudits please review again to see whether you have any further comments. I would like to merge this on Monday next week. thanks

@Emily-Jiang Emily-Jiang merged commit f212829 into microprofile:main Apr 9, 2024
3 checks passed
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.

5 participants