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

[fileexporter] refactor to use compressed writer in fileexporter #22079

Conversation

moh-osman3
Copy link
Contributor

@moh-osman3 moh-osman3 commented May 18, 2023

Description:
[PART 1] Breaking #22021 into 2 PRs
This PR is refactoring of the fileexporter to use a compressed writer. The goal of this change is to make it easier to read compressed data in the filereceiver by using a corresponding compressed reader.

This also makes it clearer to distinguish between the two types of writes. 1. writing line by line used for json and compressed json 2. writing a chunk at a time prefixed with the chunk size

For more context see part 2: #22080

Link to tracking Issue:
Related to #13626
Supports ongoing effort to record and replay telemetry data for experimental/research purposes

Testing:
Locally running otelcontribcol and writing to a file using all supported formats (json, json+compression, proto, proto+compression) and recovering original metrics using filereceiver

Documentation:

@jmacd
Copy link
Contributor

jmacd commented May 19, 2023

This change probably needs to be documented. It's breaking in a sense, but since the file receiver has not supported the things this is breaking, I would argue it doesn't matter.

.chloggen/use-compressed-writer-in-fileexporter.yaml Outdated Show resolved Hide resolved
exporter/fileexporter/file_exporter.go Outdated Show resolved Hide resolved
exporter/fileexporter/file_exporter.go Outdated Show resolved Hide resolved
@moh-osman3 moh-osman3 force-pushed the mohosman/fileexporter-compressed-writer branch from 37d1aca to f4b742f Compare June 1, 2023 21:45
@moh-osman3 moh-osman3 requested review from codeboten and atoulme June 1, 2023 22:01
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the Stale label Jun 16, 2023
@moh-osman3 moh-osman3 force-pushed the mohosman/fileexporter-compressed-writer branch from 9204fc5 to 9ccaac8 Compare June 18, 2023 18:26
@moh-osman3
Copy link
Contributor Author

@atoulme @codeboten I refactored this change again and resolved a bug I found while testing. Would you mind reviewing again when you get the chance? Thanks!

@github-actions github-actions bot removed the Stale label Jun 19, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

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

@github-actions github-actions bot added the Stale label Jul 5, 2023
@github-actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants