exporters: add otlplogfile exporter #5743

thomasgouveia · 2024-08-27T12:29:45Z

This PR introduces an experimental otlplogfile exporter based on the following specification: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md.

For the implementation, I decided to use a buffered file writer based on the existing fileexporter available in the opentelemetry-collector to reduce I/O and improve performance. Below is benchmark result:

goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/exporters/otlp/otlplog/otlplogfile/internal/writer
cpu: Apple M1 Pro
BenchmarkBufferedWriter
BenchmarkBufferedWriter/raw-file_10485760_bytes
BenchmarkBufferedWriter/raw-file_10485760_bytes-8         	36347986	        31.49 ns/op	      48 B/op	       1 allocs/op
BenchmarkBufferedWriter/buffered-file_10485760_bytes
BenchmarkBufferedWriter/buffered-file_10485760_bytes-8    	442928779	         2.704 ns/op	       0 B/op	       0 allocs/op
BenchmarkBufferedWriter/raw-file_104857600_bytes
BenchmarkBufferedWriter/raw-file_104857600_bytes-8        	34564147	        31.50 ns/op	      48 B/op	       1 allocs/op
BenchmarkBufferedWriter/buffered-file_104857600_bytes
BenchmarkBufferedWriter/buffered-file_104857600_bytes-8   	439031508	         2.703 ns/op	       0 B/op	       0 allocs/op
BenchmarkBufferedWriter/buffered-file_1073741824_bytes
BenchmarkBufferedWriter/buffered-file_1073741824_bytes-8  	400000000	         2.681 ns/op	       0 B/op	       0 allocs/op
BenchmarkBufferedWriter/raw-file_1073741824_bytes
BenchmarkBufferedWriter/raw-file_1073741824_bytes-8       	31952744	        35.17 ns/op	      48 B/op	       1 allocs/op
BenchmarkBufferedWriter/raw-file_10737418240_bytes
BenchmarkBufferedWriter/raw-file_10737418240_bytes-8      	       1	5706859250 ns/op	      32 B/op	       2 allocs/op
BenchmarkBufferedWriter/buffered-file_10737418240_bytes
BenchmarkBufferedWriter/buffered-file_10737418240_bytes-8 	       1	5981516417 ns/op	       0 B/op	       0 allocs/op
PASS

To serialize the records to the OTLP JSON format, I used protojson.

Closes #5408

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]>

Signed-off-by: thomasgouveia <[email protected]>

CHANGELOG.md

exporters/otlp/otlplog/otlplogfile/exporter.go

dmathieu · 2024-09-02T11:48:09Z

exporters/otlp/otlplog/otlplogfile/internal/writer/writer.go

+	}
+
+	// As stated in the specification, line separator is \n.
+	// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md#json-lines-file


Suggested change

// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md#json-lines-file

// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.36.0/specification/protocol/file-exporter.md#json-lines-file

exporters/otlp/otlplog/otlplogfile/go.mod

Co-authored-by: Damien Mathieu <[email protected]>

exporters/otlp/otlplog/otlplogfile/config.go

Signed-off-by: thomasgouveia <[email protected]>

… f-add-otlplogfile-exporter

pellared · 2024-09-09T07:23:27Z

exporters/otlp/otlplog/otlplogfile/config.go

+
+// WithWriter configures the destination where the exporter should output
+// the records. By default, if not specified, stdout is used.
+func WithWriter(w io.WriteCloser) Option {


The exporter must not be responsible for closing the writer (like e.g. stdoutlog)
Would you want to close stdout? Or what if someone uses one file for multiple purposes?
The caller should be responsible for closing the writer.

pellared · 2024-09-09T07:23:39Z

exporters/otlp/otlplog/otlplogfile/config.go

+
+// WithFile configures a file where the records will be exported.
+// An error is returned if the file could not be created or opened.
+func WithFile(path string) Option {


We do not need such option.

pellared · 2024-09-09T07:24:36Z

exporters/otlp/otlplog/otlplogfile/config.go

+)
+
+// Option configures a field of the configuration or return an error if needed.
+type Option func(*config) (*config, error)


Please implement Option as described in https://github.com/open-telemetry/opentelemetry-go/blob/main/CONTRIBUTING.md#option. Consistency makes the maintenance easier for us.

pellared · 2024-09-09T07:26:14Z

exporters/otlp/otlplog/otlplogfile/internal/writer/writer.go

+)
+
+// Writer writes data to the configured io.WriteCloser.
+// It is buffered to reduce I/O operations to improve performance.


Processors e.g. BatchProcessor (see here) are responsible for buffering. Can we remove the WithFlushInterval option and startFlusher? The user can do it by himself, It is good enough if exporter.ForceFlush will call a Flush.

pellared · 2024-09-09T07:37:23Z

CHANGELOG.md

@@ -8,6 +8,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

 ## [Unreleased]

+### Added
+
+- Add `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlplogfile ` experimental logs exporter. (#5743)


Suggested change

- Add `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlplogfile ` experimental logs exporter. (#5743)

- Add `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlplogfile` experimental OTLP logs file exporter. (#5743)

MrAlias · 2024-09-10T18:51:25Z

I do not think we should accept this change. It adds an exporter that has not been stabilized at the specification level. This addition will add churn to our stabilization efforts of the log signal.

I do not see why this cannot live in a fork as a PoC for the specification, or as an independent repository with its own maintainers outside of OTel. Adding this to the OTel development teams maintenance burden does not seem warranted at this time.

dashpole · 2024-09-12T17:22:41Z

From the SIG meeting:

This is currently in development, and we aren't required to implement it
This is a large amount of code, and will likely be a burden on maintainers as it evolves
We would prefer if this lived in its own repository until it is stabilized and required for us to implement

pellared · 2024-11-08T08:34:48Z

Changing to draft as there is not work here currently.

thomasgouveia force-pushed the f-add-otlplogfile-exporter branch from 02d2dab to 64ee265 Compare August 27, 2024 12:33

thomasgouveia added 4 commits August 27, 2024 14:40

docs(open-telemetry#5408): add example file

7dde8dc

Signed-off-by: thomasgouveia <[email protected]>

docs(open-telemetry#5408): update CHANGELOG

0f3e7c8

Signed-off-by: thomasgouveia <[email protected]>

docs(open-telemetry#5408): update CHANGELOG

37c4f5e

Signed-off-by: thomasgouveia <[email protected]>

thomasgouveia force-pushed the f-add-otlplogfile-exporter branch from 64ee265 to 37c4f5e Compare August 27, 2024 12:40

thomasgouveia marked this pull request as ready for review August 27, 2024 12:42

thomasgouveia requested review from MrAlias, XSAM, dashpole, MadVikingGod, pellared, hanyuancheung and dmathieu as code owners August 27, 2024 12:42

dmathieu reviewed Sep 2, 2024

View reviewed changes

thomasgouveia and others added 2 commits September 3, 2024 09:08

chore: apply suggestions

cc10281

Co-authored-by: Damien Mathieu <[email protected]>

chore: apply suggestions

fc7212e

Co-authored-by: Damien Mathieu <[email protected]>

pellared requested a review from dmathieu September 5, 2024 09:06

pellared reviewed Sep 5, 2024

View reviewed changes

exporters/otlp/otlplog/otlplogfile/config.go Outdated Show resolved Hide resolved

dmathieu approved these changes Sep 5, 2024

View reviewed changes

thomasgouveia added 2 commits September 6, 2024 14:32

feat(open-telemetry#5408): use stdout as default output

7244adc

Signed-off-by: thomasgouveia <[email protected]>

Merge branch 'main' of github.com:thomasgouveia/opentelemetry-go into…

100dc15

… f-add-otlplogfile-exporter

pellared reviewed Sep 9, 2024

View reviewed changes

pellared mentioned this pull request Sep 12, 2024

Add otlplogfile exporter #5408

Closed

pellared marked this pull request as draft November 8, 2024 08:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

exporters: add otlplogfile exporter #5743

exporters: add otlplogfile exporter #5743

thomasgouveia commented Aug 27, 2024 •

edited

Loading

dmathieu Sep 2, 2024

pellared Sep 9, 2024

pellared Sep 9, 2024

pellared Sep 9, 2024

pellared Sep 9, 2024 •

edited

Loading

pellared Sep 9, 2024 •

edited

Loading

MrAlias commented Sep 10, 2024

dashpole commented Sep 12, 2024

pellared commented Nov 8, 2024 •

edited

Loading

	// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md#json-lines-file
	// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.36.0/specification/protocol/file-exporter.md#json-lines-file

	- Add `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlplogfile ` experimental logs exporter. (#5743)
	- Add `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlplogfile` experimental OTLP logs file exporter. (#5743)

exporters: add otlplogfile exporter #5743

Are you sure you want to change the base?

exporters: add otlplogfile exporter #5743

Conversation

thomasgouveia commented Aug 27, 2024 • edited Loading

dmathieu Sep 2, 2024

Choose a reason for hiding this comment

pellared Sep 9, 2024

Choose a reason for hiding this comment

pellared Sep 9, 2024

Choose a reason for hiding this comment

pellared Sep 9, 2024

Choose a reason for hiding this comment

pellared Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

pellared Sep 9, 2024 • edited Loading

Choose a reason for hiding this comment

MrAlias commented Sep 10, 2024

dashpole commented Sep 12, 2024

pellared commented Nov 8, 2024 • edited Loading

thomasgouveia commented Aug 27, 2024 •

edited

Loading

pellared Sep 9, 2024 •

edited

Loading

pellared Sep 9, 2024 •

edited

Loading

pellared commented Nov 8, 2024 •

edited

Loading