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

Is OTLP file exporter required for SDKs? #3817

Open
jack-berg opened this issue Jan 11, 2024 · 10 comments
Open

Is OTLP file exporter required for SDKs? #3817

jack-berg opened this issue Jan 11, 2024 · 10 comments
Assignees
Labels
spec:protocol Related to the specification/protocol directory triage:accepted:ready-with-sponsor Ready to be implemented and has a specification sponsor assigned

Comments

@jack-berg
Copy link
Member

The OTLP File exporter seems like it could be an implementation requirement for SDKs, but on closer inspection:

  • It does not appear as a known value for OTEL_TRACES_EXPORTER, OTEL_METRICS_EXPORTER, OTEL_LOGS_EXPORTER
  • It only appears in the spec compliance matrix under logs. Its not implemented by any SDKs so this may have been a mistake.

On one hand, it would be nice to be able to configure SDKs to export to files. On the other hand, supporting file export requires a variety of additional un-specified configuration options including the the name of the file, whether a file rotation strategy is used, and more. Additionally, supporting this in the SDK when the functionality exists in the collector duplicates effort.

@jack-berg jack-berg added the spec:protocol Related to the specification/protocol directory label Jan 11, 2024
@reyang reyang added the [label deprecated] triaged-needmoreinfo [label deprecated] The issue is triaged - the OTel community needs more information to decide label Jan 24, 2024
@austinlparker austinlparker added triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted and removed [label deprecated] triaged-needmoreinfo [label deprecated] The issue is triaged - the OTel community needs more information to decide labels Apr 30, 2024
@austinlparker
Copy link
Member

It would be good if we could get more specific feedback about the intended usage, use cases, and related work on this.

@zeitlinger
Copy link
Member

The intended usage is to be to counterpart of the otlpjsonfilereceiver component - which is currently under development:

open-telemetry/opentelemetry-collector-contrib#33846

@zeitlinger
Copy link
Member

Regarding the spec, I'd propose

OTEL_LOGS_EXPORTER=stdout

and

OTEL_LOGS_EXPORTER=file
OTEL_LOGS_EXPORTER_FILE=/path/to/file

wdyt?

@austinlparker
Copy link
Member

Which SIGs would this impact? All of them (outside of collector?)

@zeitlinger
Copy link
Member

Which SIGs would this impact? All of them (outside of collector?)

yes, all language sigs

@pellared
Copy link
Member

pellared commented Jul 24, 2024

@zeitlinger, I would like to have otlp in the name.

I propose to

Add otlpstdout, otlpstderr, otlpfile to known values of OTEL_TRACES_EXPORTER, OTEL_METRICS_EXPORTER, OTEL_LOGS_EXPORTER`.

Add OTLP_FILE_EXPORTER_PATH, OTLP_FILE_TRACES_EXPORTER_PATH, OTLP_FILE_METRICS_EXPORTER_PATH, OTLP_FILE_LOGS_EXPORTER_PATH which works only for otlpfile (based on https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#configuration-options but names have a little different convention to have the names shorter). The per-signals env vars take precedence over OTLP_FILE_EXPORTER_PATH.

@zeitlinger
Copy link
Member

good point, so the question is

  • OTLP_FILE_METRICS_EXPORTER_PATH vs. OTEL_EXPORTER_OTLP_METRICS_FILE (or _PATH)
  • OTLP_FILE_EXPORTER_PATH vs. OTEL_EXPORTER_OTLP_FILE (or _PATH)

same number of chars, so I'd prefer the second, more consistent option

@pellared
Copy link
Member

pellared commented Sep 19, 2024

@open-telemetry/technical-committee, can you please "re-triage" the issue given the community feedback we got #4183 (comment) + comments below?

I can be assigned as a sponsor as I am already trying to assist @zeitlinger.

@github-actions github-actions bot added the triage:followup Needs follow up during triage label Sep 19, 2024
@trask trask added triage:accepted:ready-with-sponsor Ready to be implemented and has a specification sponsor assigned and removed triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted triage:followup Needs follow up during triage labels Sep 24, 2024
@jsuereth
Copy link
Contributor

jsuereth commented Nov 5, 2024

I realize I'm a bit late to this party (around file export). I just want to call out that I think writing JSON is a "simple" exporter, but possibly not the most efficient way to do file-based communication between Process -> Collector.

I put together a quick (trace-only) prototype e.g. of a more sophisticated file format (designed as an IPC mechanism): https://github.com/jsuereth/otlp-mmap/

I'm not sure this is the right issue for this discussion, but I do think we should create a strategy for this kind of architecture:

flowchart LR
SDK-->|writes| file
Collector-->|reads| file
Collector-->|otlp| Backend
Loading

I do think a naive "json" export is fine, but I think we should also be considering something that allows us to "get data off process" as fast as possible, e.g. for dealing with crash scenarios.

Posting here to better understand if this is the intended use case of the file exporter and if we think we should be designing a more efficient / robust solution that would eventually be the "default recommended" path for writing to files.

@zeitlinger
Copy link
Member

I think it's great to have more eyes on the topic - and I agree that the current iteration I'm working on (json over stdout) can probably be improved - but I still think it's a good first step as it unlocks many use cases and is easy to use with k8s.

jsuereth pushed a commit that referenced this issue Nov 26, 2024
Towards
#3817

## Changes

Add OTLP Stdout exporter.

For non-trivial changes, follow the [change proposal
process](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CONTRIBUTING.md#proposing-a-change).

* [x] Related issues
#3817
~* [ ] Related [OTEP(s)](https://github.com/open-telemetry/oteps) #~
* [x] Links to the prototypes:
open-telemetry/opentelemetry-java#6632
* [x]
[`CHANGELOG.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CHANGELOG.md)
file updated for non-trivial changes
~* [ ]
[`spec-compliance-matrix.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/spec-compliance-matrix.md)
updated if necessary~
* [x] reference implementation
open-telemetry/opentelemetry-java#6632

---------

Co-authored-by: Robert Pająk <[email protected]>
Co-authored-by: Cyrille Le Clerc <[email protected]>
Co-authored-by: Cijo Thomas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:protocol Related to the specification/protocol directory triage:accepted:ready-with-sponsor Ready to be implemented and has a specification sponsor assigned
Projects
Status: Spec - Accepted
Development

No branches or pull requests

8 participants