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

OTLP file exporter #2482

Closed
owent opened this issue Jan 10, 2024 · 12 comments · Fixed by #2540
Closed

OTLP file exporter #2482

owent opened this issue Jan 10, 2024 · 12 comments · Fixed by #2540
Assignees
Labels
area:exporter:otlp OpenTelemetry Protocol (OTLP) Exporter do-not-stale triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@owent
Copy link
Member

owent commented Jan 10, 2024

Is your feature request related to a problem?

According to
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/file-exporter.md#file-storage-requirements .

It we have file exporter, some projects need not depend on openssl, curl, or gRPC anymore.

Describe the solution you'd like

Use OTLP utils to convert data to JSON format, just like it in OTLP HTTP exporter.

Additional context

There are some questions I want to discuss before implementing it.
In practice, we usually have log rotate to control the disk usage, but the specification does not say anything about it.
Can I add some extensions to it?
For example, if we configure the file pattern as %Y-%m-%d/otlp.%N.log and set the rotate size to three, we want to create a directory every day and rotate output file name as otlp.0.log, otlp.1.log and otlp.2.log. And we may also set alias name %Y-%m-%d/otlp.log so the otlp.log will always point to the latest one?

@owent owent added area:exporter:otlp OpenTelemetry Protocol (OTLP) Exporter do-not-stale labels Jan 10, 2024
@owent owent self-assigned this Jan 10, 2024
@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jan 10, 2024
@marcalff
Copy link
Member

For systems in production that generate a lot of telemetry, exporting data to a file has value.

A possible use case:

  • always export to a file
  • when no incident occurs, let the data rot on the file system, and be removed (by an external cleanup process)
  • when an incident occurs, archive the data, and upload the data to a telemetry backend for inspection

This way, the backend is not flooded constantly.

@owent

This is an important feature in my opinion, thanks for taking this on.

File rotation is very desirable.

@owent
Copy link
Member Author

owent commented Jan 10, 2024

We have a internal prometheus file exporter(which can be push to opentelemetry-cpp-contrib later after we test foe some time).
The options are like this:

/**
 * Struct to hold Prometheus exporter options.
 * @note Available placeholder for file_pattern and alias_pattern:
 *     %Y:  writes year as a 4 digit decimal number
 *     %y:  writes last 2 digits of year as a decimal number (range [00,99])
 *     %m:  writes month as a decimal number (range [01,12])
 *     %j:  writes day of the year as a decimal number (range [001,366])
 *     %d:  writes day of the month as a decimal number (range [01,31])
 *     %w:  writes weekday as a decimal number, where Sunday is 0 (range [0-6])
 *     %H:  writes hour as a decimal number, 24 hour clock (range [00-23])
 *     %I:  writes hour as a decimal number, 12 hour clock (range [01,12])
 *     %M:  writes minute as a decimal number (range [00,59])
 *     %S:  writes second as a decimal number (range [00,60])
 *     %F:  equivalent to "%Y-%m-%d" (the ISO 8601 date format)
 *     %T:  equivalent to "%H:%M:%S" (the ISO 8601 time format)
 *     %R:  equivalent to "%H:%M"
 *     %N:  rotate index, start from 0
 *     %n:  rotate index, start from 1
 */
struct PrometheusFileExporterOptions {
  std::string file_pattern = "%Y-%m-%d/prometheus.%N.log";
  std::string alias_pattern = "%Y-%m-%d/prometheus.log";
  std::chrono::microseconds flush_interval = std::chrono::microseconds{30000000};
  std::size_t flush_count = 256;
  std::size_t file_size = 20 * 1024 * 1024;
  std::size_t rotate_size = 3;

  inline PrometheusFileExporterOptions() noexcept {}
};

Do you think I can bring all the options to OTLP file exporters?

@marcalff marcalff added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 10, 2024
@lalitb
Copy link
Member

lalitb commented Jan 11, 2024

This is a good feature to add.

It is good to have a look into the configuration option for the OTLP Collector file exporter too - https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.92.0/exporter/fileexporter/README.md.

Regarding log rotation, we can also leave it to developers to use more sophisticated tooling like logrotate or a simple custom script running regularly as cron to handle that.

@owent
Copy link
Member Author

owent commented Jan 12, 2024

It is good to have a look into the configuration option for the OTLP Collector file exporter too - https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.92.0/exporter/fileexporter/README.md.

This file exporter uses time to rotate files, and does not allow user to set the rule to generate file names or creating directories. I may be useful when we have a lot of outputs for a long time. And it's difficult to support compression without more dependencies using C++. Do you think we should depend zlb,zstd,lz4 or other libraries to just finish this feature? Or can we consider this feature in the future?

Regarding log rotation, we can also leave it to developers to use more sophisticated tooling like logrotate or a simple custom script running regularly as cron to handle that.

The spec says we should support export to stdout, I think we can use this way to let users to decide which tools to handle these files.

@marcalff
Copy link
Member

@owent
Copy link
Member Author

owent commented Feb 1, 2024

See related:

Thanks, I will follow this issue.

@perhapsmaple
Copy link
Contributor

perhapsmaple commented Feb 13, 2024

@owent I'm planning to deploy opentelemetry as the primary logging system for my organization's c++ projects, and we have a requirement for exporting logs to disk before being read by the collector. I have implemented a OtlpFileExporter currently which does the same thing as the OtlpHttpExporter, but writes the nlohmann::json to disk instead of exporting it with curl. This pipeline is rather heavy and slow compared to the existing logger.

I understand that it will be difficult to get close to the speed of plaintext logging, but is there a better way to implement a OTLP format compliant exporter that can skip the dependency on the recordable->protobuf->json conversion. Just looking for any ideas to improve.

Thanks

@owent
Copy link
Member Author

owent commented Feb 14, 2024

@owent @lalitb @marcalff I'm planning to deploy opentelemetry as the primary logging system for my organization's c++ projects, and we have a requirement for exporting logs to disk before being read by the collector. I have implemented a OtlpFileExporter currently which does the same thing as the OtlpHttpExporter, but writes the nlohmann::json to disk instead of exporting it with curl. This pipeline is rather heavy and slow compared to the existing logger.

I understand that it will be difficult to get close to the speed of plaintext logging, but is there a better way to implement a OTLP format compliant exporter that can skip the dependency on the recordable->protobuf->json conversion. Just looking for any ideas to improve.

Thanks

It would be nice if you can take this issue and push your OtlpFileExporter here if it follow specification.I'm too busy recently and only can start this after one or two weeks.

We also meet some users do not want to depend on protobuf when they use OTLP HTTP exporters.
In my understanding, maybe we can implement another group of codes to create json from Recordable directly and without reflection of protobuf.But we must write enough test codes to ensure the data structs is correct because the proto are changing over time.
To improve performance,we can use faster json library to handle serialization such as rapidjson or simdjson.
Do you have any idea?

@perhapsmaple
Copy link
Contributor

The OtlpFileExporter that I currently use is nothing more than a slightly modified OtlpHttpExporter. Currently, it does something like this:

nlohmann::json json_request;
ConvertGenericMessageToJson(json_request, message, options_);
writer_ << json_request;

I write all of the recordables to a file, and read it later with the otlpjsonfilereceiver from opentelemetry-collector-contrib. All of the file handline is done by an external module in my case, but it would be enough to make a small wrapper over std::ofstream. Due to relying on the OtlpHttpExporter, we also have the dependency on protobuf currently. I'm currently exploring alternative implementations to generate the JSON directly from the Recordable but as you said, it is difficult to maintain compatibility with opentelemetry-proto.

Regarding performance, the nlohmann::json library is quite slow compared to rapidjson and simdjson. We gained an approximate 50% performance boost in an ancient Ivy Bridge server switching from nlohmann::json to rapidjson. I assume simdjson would be even faster, especially with newer processors. I will try creating a simdjson implementation and will benchmark all three this weekend to provide a bit more clarity regarding this.

I'm happy to submit a PR to add the OtlpFileExporter to opentelemetry::exporters::otlp. @lalitb @marcalff Any thoughts regarding this?

Thanks

@owent
Copy link
Member Author

owent commented Feb 15, 2024

I have implemented a prometheus file exporter before, which also support file rotate and has a alias name which always link to the latest file.Just like the comment in struct PrometheusFileExporterOptions above. It also support to interval flush the output device, so we can find the output even there is no more data to export for a long time.
The file path can be set by datetime and maybe create directories over time.For example, set file_pattern to %Y-%m-%d/prometheus.%N.log, we will create a directory every day.

This prometheus file exporter works for a long time in our internal system, and which I planed before is implementing a OTLP File exporter based on this prometheus file exporter. Maybe we can create a branch in which you can push your codes to support origin std:ostream and I can commit my codes to support file rotation later?

BTW: Do you think we can raise another issue to track the discussion about performance optimization and json library?

@perhapsmaple
Copy link
Contributor

I'm sorry about that @owent. I wasn't able to get permission from higher ups in time to submit a PR earlier.

@owent
Copy link
Member Author

owent commented Feb 19, 2024

I'm sorry about that @owent. I wasn't able to get permission from higher ups in time to submit a PR earlier.

I have just create a draft PR yestoday and has a simple implementation about write OTLP json to ostream. And I will write some tests later. Could you please help to review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:exporter:otlp OpenTelemetry Protocol (OTLP) Exporter do-not-stale triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants