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 otlplogfile exporter #5408

Closed
pellared opened this issue May 23, 2024 · 5 comments · May be fixed by #5743
Closed

Add otlplogfile exporter #5408

pellared opened this issue May 23, 2024 · 5 comments · May be fixed by #5743
Assignees
Labels
area:logs Part of OpenTelemetry logs enhancement New feature or request pkg:exporter:otlp Related to the OTLP exporter package

Comments

@pellared
Copy link
Member

Implement an experimental OTLP Log File exporter based on https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md

@pellared pellared added the enhancement New feature or request label May 23, 2024
@pellared pellared added area:logs Part of OpenTelemetry logs pkg:exporter:otlp Related to the OTLP exporter package labels May 23, 2024
@dmathieu dmathieu changed the title Add otlplogfile expoter Add otlplogfile exporter May 23, 2024
MrAlias added a commit that referenced this issue Jun 11, 2024
The reason for this improvement (apart that, in general, it is good to
have better performance) is there may be good use case to use a
`SimpleProcessor` to emit logs efficiently to standard output. It could
be one of the most efficient solutions (from application performance
perspective) and thanks to such configuration the user would not lose
any logs if the application suddenly crashes. For instance, a useful
configuration could be a simple processor with an OTLP file exporter
(#5408).

I think we might consider changing the following portion of
`SimpleProcessor` documentation (but I would prefer to do it as separate
PR):

> // This Processor is not recommended for production use. The
synchronous
// nature of this Processor make it good for testing, debugging, or
// showing examples of other features, but it can be slow and have a
high
// computation resource usage overhead. [NewBatchProcessor] is
recommended
// for production use instead.

```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/log
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
                                    │   old.txt   │               new.txt                │
                                    │   sec/op    │    sec/op     vs base                │
Processor/Simple-16                   449.4n ± 7%   156.2n ±  5%  -65.25% (p=0.000 n=10)
Processor/ModifyTimestampSimple-16    468.0n ± 6%   171.3n ± 15%  -63.40% (p=0.000 n=10)
Processor/ModifyAttributesSimple-16   515.8n ± 3%   233.2n ±  8%  -54.77% (p=0.000 n=10)
geomean                               476.9n        184.1n        -61.40%

                                    │   old.txt   │                 new.txt                 │
                                    │    B/op     │    B/op     vs base                     │
Processor/Simple-16                    417.0 ± 0%     0.0 ± 0%  -100.00% (p=0.000 n=10)
Processor/ModifyTimestampSimple-16     417.0 ± 0%     0.0 ± 0%  -100.00% (p=0.000 n=10)
Processor/ModifyAttributesSimple-16   465.00 ± 0%   48.00 ± 0%   -89.68% (p=0.000 n=10)
geomean                                432.4                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

                                    │  old.txt   │                 new.txt                 │
                                    │ allocs/op  │ allocs/op   vs base                     │
Processor/Simple-16                   1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)
Processor/ModifyTimestampSimple-16    1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)
Processor/ModifyAttributesSimple-16   2.000 ± 0%   1.000 ± 0%   -50.00% (p=0.000 n=10)
geomean                               1.260                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean
```

---------

Co-authored-by: Tyler Yahn <[email protected]>
OrHayat pushed a commit to OrHayat/opentelemetry-go that referenced this issue Jun 23, 2024
…lemetry#5493)

The reason for this improvement (apart that, in general, it is good to
have better performance) is there may be good use case to use a
`SimpleProcessor` to emit logs efficiently to standard output. It could
be one of the most efficient solutions (from application performance
perspective) and thanks to such configuration the user would not lose
any logs if the application suddenly crashes. For instance, a useful
configuration could be a simple processor with an OTLP file exporter
(open-telemetry#5408).

I think we might consider changing the following portion of
`SimpleProcessor` documentation (but I would prefer to do it as separate
PR):

> // This Processor is not recommended for production use. The
synchronous
// nature of this Processor make it good for testing, debugging, or
// showing examples of other features, but it can be slow and have a
high
// computation resource usage overhead. [NewBatchProcessor] is
recommended
// for production use instead.

```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/log
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
                                    │   old.txt   │               new.txt                │
                                    │   sec/op    │    sec/op     vs base                │
Processor/Simple-16                   449.4n ± 7%   156.2n ±  5%  -65.25% (p=0.000 n=10)
Processor/ModifyTimestampSimple-16    468.0n ± 6%   171.3n ± 15%  -63.40% (p=0.000 n=10)
Processor/ModifyAttributesSimple-16   515.8n ± 3%   233.2n ±  8%  -54.77% (p=0.000 n=10)
geomean                               476.9n        184.1n        -61.40%

                                    │   old.txt   │                 new.txt                 │
                                    │    B/op     │    B/op     vs base                     │
Processor/Simple-16                    417.0 ± 0%     0.0 ± 0%  -100.00% (p=0.000 n=10)
Processor/ModifyTimestampSimple-16     417.0 ± 0%     0.0 ± 0%  -100.00% (p=0.000 n=10)
Processor/ModifyAttributesSimple-16   465.00 ± 0%   48.00 ± 0%   -89.68% (p=0.000 n=10)
geomean                                432.4                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean

                                    │  old.txt   │                 new.txt                 │
                                    │ allocs/op  │ allocs/op   vs base                     │
Processor/Simple-16                   1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)
Processor/ModifyTimestampSimple-16    1.000 ± 0%   0.000 ± 0%  -100.00% (p=0.000 n=10)
Processor/ModifyAttributesSimple-16   2.000 ± 0%   1.000 ± 0%   -50.00% (p=0.000 n=10)
geomean                               1.260                    ?                       ¹ ²
¹ summaries must be >0 to compute geomean
² ratios must be >0 to compute geomean
```

---------

Co-authored-by: Tyler Yahn <[email protected]>
@MrAlias MrAlias moved this to Backlog in Go: Logs (Post-GA) Aug 1, 2024
@thomasgouveia
Copy link

thomasgouveia commented Aug 8, 2024

Hi @pellared!

Before doing this one, as stated in #5500, maybe it will be necessary to introduce something that can serialize the data into the OpenTelemetry JSON format.

Because as you mentioned here, we can add a public function into a otlplogfile package, but is it coherent with what the package is responsible for? I mean that IMHO, the otlplogfile package should be responsible for appending log records to a JSON file.

If we extract the serialization part into another internal (or public, if it makes sense) package such as otlpserde (naming is just an example), we can use a dependency into otlplogfile and stdoutlog, so the format will be consistent across the different implementations.

What do you think about it?

@pellared
Copy link
Member Author

pellared commented Aug 8, 2024

stdoutlog is intended to be used mainly for debugging therefore it aims for human-readability.

otlplogfile is on the other hand MUST serialize telemetry in computer-readable format as defined in the specification.

I suggest not extracting any separate package until necessary.

thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
This commit adds a new experimental exporter `otlplogfile`, that outputs log records to a JSON line file.
It is based on the following specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md

Signed-off-by: thomasgouveia <[email protected]>
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
This commit adds a new experimental exporter `otlplogfile`, that outputs log records to a JSON line file.
It is based on the following specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md

Signed-off-by: thomasgouveia <[email protected]>
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
This commit adds a new experimental exporter `otlplogfile`, that outputs log records to a JSON line file.
It is based on the following specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md

Signed-off-by: thomasgouveia <[email protected]>
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
This commit adds a new experimental exporter `otlplogfile`, that outputs log records to a JSON line file.
It is based on the following specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md

Signed-off-by: thomasgouveia <[email protected]>
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
This commit adds a new experimental exporter `otlplogfile`, that outputs log records to a JSON line file.
It is based on the following specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md

Signed-off-by: thomasgouveia <[email protected]>
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
This commit adds a new experimental exporter `otlplogfile`, that outputs log records to a JSON line file.
It is based on the following specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md

Signed-off-by: thomasgouveia <[email protected]>
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
This commit adds a new experimental exporter `otlplogfile`, that outputs log records to a JSON line file.
It is based on the following specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md

Signed-off-by: thomasgouveia <[email protected]>
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
This commit adds a new experimental exporter `otlplogfile`, that outputs log records to a JSON line file.
It is based on the following specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md

Signed-off-by: thomasgouveia <[email protected]>
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Aug 27, 2024
thomasgouveia added a commit to thomasgouveia/opentelemetry-go that referenced this issue Sep 6, 2024
@pellared
Copy link
Member Author

During last SIG meeting (#5743 (comment)) we decided that we do not want to maintain such exporter until it is stable.

@thomasgouveia, it may be worth to move the exporter you have created in #5743 to a separate repository and maintain it there. I can help you in maintaining it.

@pellared pellared closed this as not planned Won't fix, can't repro, duplicate, stale Sep 12, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in Go: Logs (Post-GA) Sep 12, 2024
@thomasgouveia
Copy link

@pellared may it be located in the contrib repository? Or the contrib focuses only on stable implementation?

If we can't put it in the contrib repository, I can transfer it to a repository on my account, until the specification is stable.

@pellared
Copy link
Member Author

pellared commented Sep 13, 2024

During SIG it was recommended to not put it to contrib as well yet.

I can transfer it to a repository on my account, until the specification is stable.

Let's do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:logs Part of OpenTelemetry logs enhancement New feature or request pkg:exporter:otlp Related to the OTLP exporter package
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants