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 OTLP stdout exporter #4183

Merged
merged 22 commits into from
Nov 26, 2024
Merged

Add OTLP stdout exporter #4183

merged 22 commits into from
Nov 26, 2024

Conversation

zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Aug 8, 2024

Towards #3817

Changes

Add OTLP Stdout exporter.

For non-trivial changes, follow the change proposal process.

Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

How does this relate to the existing console/stdout1 and file exporters? This seems at best overlapping, but at worst duplicative of already specified exporters.

Footnotes

  1. https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk_exporters/stdout.md

specification/common/otlp-stdout.md Outdated Show resolved Hide resolved
specification/common/otlp-stdout.md Outdated Show resolved Hide resolved
specification/common/otlp-stdout.md Outdated Show resolved Hide resolved
specification/common/otlp-stdout.md Outdated Show resolved Hide resolved
specification/common/otlp-stdout.md Outdated Show resolved Hide resolved
@pellared
Copy link
Member

pellared commented Aug 8, 2024

What about standard error and and file as output destinations?

@zeitlinger
Copy link
Member Author

How does this relate to the existing console/stdout and file exporters? This seems at best overlapping, but at worst duplicative of already specified exporters.

@zeitlinger
Copy link
Member Author

@pellared @MrAlias can you check again?

@cijothomas
Copy link
Member

Fixes #3817

The linked issue is about OTLP File exporter, but this seems to talking about stdout exporter... Did I misread this?

@zeitlinger
Copy link
Member Author

Fixes #3817

The linked issue is about OTLP File exporter, but this seems to talking about stdout exporter... Did I misread this?

otlp-stdout is part of the file exporter

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Aug 21, 2024
@zeitlinger
Copy link
Member Author

@pellared @MrAlias can you check again?

@github-actions github-actions bot removed the Stale label Aug 22, 2024
@zeitlinger
Copy link
Member Author

@cijothomas can you check again?

Copy link

github-actions bot commented Sep 3, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Sep 3, 2024
@zeitlinger
Copy link
Member Author

still working on the pr

specification/protocol/file-exporter.md Outdated Show resolved Hide resolved
specification/protocol/file-exporter.md Outdated Show resolved Hide resolved
specification/protocol/file-exporter.md Outdated Show resolved Hide resolved
specification/configuration/sdk-environment-variables.md Outdated Show resolved Hide resolved
specification/protocol/file-exporter.md Outdated Show resolved Hide resolved
specification/configuration/sdk-environment-variables.md Outdated Show resolved Hide resolved
specification/configuration/sdk-environment-variables.md Outdated Show resolved Hide resolved
specification/configuration/sdk-environment-variables.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
specification/configuration/sdk-environment-variables.md Outdated Show resolved Hide resolved
specification/configuration/sdk-environment-variables.md Outdated Show resolved Hide resolved
@cyrille-leclerc
Copy link
Member

cyrille-leclerc commented Sep 10, 2024

As discussed with @zeitlinger, this capability to export standardized OTel logs in files that can be seamlessly ingested by the OTel Collector is a major improvement for OTel logs.

That being said, I would be interested in understanding better the developer experience, how this OTEL_LOGS_EXPORTER=otlp/stdout config option will co-exist with the config of logging framework like Logback in Java?

Taking the example of a Java Spring Boot developer, I'll typically use the default logs configuration that emits unstructured logs in stdout (example below) and, if I want to modify this, I'll modify this config through Spring's application.properties and logback-spring.xml files (see reference docs here).

If I want to modify the stdout output of my Spring Boot application to adopt OTel JSON logging to replace the unstructured logs,

  1. I'll expect to have to modify these Spring Boot config files for logging: application.properties and logback-spring.xml
  2. I'll have to modify these Spring Boot config files to disable the unstructured log messages that are outputted in stdout.

I think that Elastic has done this very well with the Elastic ECS Logging framework. In addition to a manual config, Elastic has an experimental config flag log_ecs_reformatting do seamlessly change the logger config and disable the pre-existing config in favor of the ECS/JSON one. Maybe @AlexanderWert could share some lessons learned.

I'm wondering if we could start with implementing a few extensions of popular logging frameworks (LogBack, Log4j, Pino...) that developers would manually use in their logs config.This could be a natural extension of the OTLP gRPC and HTTP appenders we have implemented so far (eg [here](Appender Instrumentation for Logback version 1.0 and higher
))

I would imagine to the following developer experience for Spring Boot app:

  • export OTEL_LOGS_EXPORTER=none to disable the otlp http output
  • Edit logback-spring.xml to:
<configuration>
  <appender name="console" class="ch.qos.logback.core.ConsoleAppender">
     <!-- below is just an idea, don't copy paste this fragment -->
     <encoder class="io.opentelemetry.....OtlpJsonEncoder">
  </appender>
  ...
  <root level="INFO">
    <appender-ref ref="console"/>
  </root>

</configuration>

Example Spring Boot console logs (Spring docs here):

2024-08-22T16:59:59.387Z  INFO 109360 --- [myapp] [           main] o.s.b.d.f.logexample.MyApplication       : Starting MyApplication using Java 17.0.12 with PID 109360 (/opt/apps/myapp.jar started by myuser in /opt/apps/)
2024-08-22T16:59:59.410Z  INFO 109360 --- [myapp] [           main] o.s.b.d.f.logexample.MyApplication       : No active profile set, falling back to 1 default profile: "default"
2024-08-22T17:00:02.105Z  INFO 109360 --- [myapp] [           main] o.s.b.w.embedded.tomcat.TomcatWebServer  : Tomcat initialized with port 8080 (http)
2024-08-22T17:00:02.133Z  INFO 109360 --- [myapp] [           main] o.apache.catalina.core.StandardService   : Starting service [Tomcat]
2024-08-22T17:00:02.133Z  INFO 109360 --- [myapp] [           main] o.apache.catalina.core.StandardEngine    : Starting Servlet engine: [Apache Tomcat/10.1.28]
2024-08-22T17:00:02.296Z  INFO 109360 --- [myapp] [           main] o.a.c.c.C.[Tomcat].[localhost].[/]       : Initializing Spring embedded WebApplicationContext
2024-08-22T17:00:02.301Z  INFO 109360 --- [myapp] [           main] w.s.c.ServletWebServerApplicationContext : Root WebApplicationContext: initialization completed in 2647 ms
2024-08-22T17:00:03.219Z  INFO 109360 --- [myapp] [           main] o.s.b.w.embedded.tomcat.TomcatWebServer  : Tomcat started on port 8080 (http) with context path '/'
2024-08-22T17:00:03.250Z  INFO 109360 --- [myapp] [           main] o.s.b.d.f.logexample.MyApplication       : Started MyApplication in 5.793 seconds (process running for 7.164)

@martinjt
Copy link
Member

I feel this is a bad direction for OpenTelemetry in general.

stdout for debugging is a valid usecase, but we should strongly encourage people to use OTLP and push the data to an endpoint (ideally a collector). We should not be promoting push data to the console/stdout and using another tool to pull that data and ingest it. I understand that K8s has done it this way in the past, and that 12 factor apps popularised this approach, but now we have a standardised protocol and we don't need that now.

From inefficiencies in some languages, to issues with polling for that data, it's just not something that we should push for when we have a lot of alternatives like OTLP and collector, that are part of the OpenTelemetry project.

All the requests I've seen from this in the .NET world have been from people not wanting to move from other technologies like fluent etc. and not that it's a better way to work.

I can understand a desire to have a console logging output that formats the log line with the template to produce the human readable string, but not one who's purpose is to provide a format for other tools to use. This will fragment the ecosystem further than it already is.

Right now, we have a path that we push people down. Use the OTLP exporter to push data to an endpoint that accepts OTLP. If we push down this route as a production usecase, that will become diluted with vendors saying "use our agent and configure OpenTelemetry to push to stdout", and still claim that they support OpenTelemetry. Especially if this is then supported for Metrics and Traces too. That will actively harm OpenTelemetry as a project, when we're trying make the developer experience better.

@zeitlinger
Copy link
Member Author

@zeitlinger It'd be great to capture some of the "why" comments from this PR into https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md#opentelemetry-protocol-file-exporter itself for an easy reference in future, "why we need a spec for non OTLP exporter".

@cijothomas done

Co-authored-by: Cyrille Le Clerc <cyrille@cyrilleleclerc.com>
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

I think this is a good step forward for users, but I also have concerns around efficiency and scalability of serialized JSON to file. I think we CAN do much better, but let's not block progress for perfection.

@jsuereth jsuereth merged commit 79380c6 into open-telemetry:main Nov 26, 2024
6 checks passed
@zeitlinger zeitlinger deleted the otlp-stdout branch November 27, 2024 07:46
@carlosalberto carlosalberto mentioned this pull request Dec 6, 2024
carlosalberto added a commit that referenced this pull request Dec 13, 2024
December 2024 Release.

Mostly in-development additions, e.g.

*
#4183
*
#4295
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.