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.

Sign up for GitHub

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

Jump to bottom

[exporter/fileexporter] Enable appending as an option instead of always truncating #31364

Closed
OverOrion opened this issue Feb 21, 2024 · 9 comments
Closed

[exporter/fileexporter] Enable appending as an option instead of always truncating #31364

OverOrion opened this issue Feb 21, 2024 · 9 comments
Assignees
@OverOrion
Labels
enhancement New feature or request exporter/file

Comments

@OverOrion
Copy link
Contributor

Component(s)

exporter/file

Is your feature request related to a problem? Please describe.

It might be desirable to let the user configure the fileexporter's file opening mode so in the event of a restart it would continue writing to the same file without losing the previous progress.

Describe the solution you'd like

I think a simple write_mode: append / truncate or append: true configuration field would be the way to go.

Describe alternatives you've considered

Using an external script to make a backup of the currently written file and only after that should the collector be restarted.

Additional context

If you agree with this proposal I would be happy to work on it once we agreed on how this option should be implemented 🚀.
@OverOrion OverOrion added enhancement New feature or request needs triage New item requiring triage labels Feb 21, 2024
@github-actions GitHub Actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@andrzej-stencel andrzej-stencel removed the needs triage New item requiring triage label Feb 21, 2024
@OverOrion OverOrion mentioned this issue Feb 21, 2024
Merged
1 task
@atingchen
Copy link
Contributor

Thank you for your issue~ @OverOrion
Are you worried that when the Collector restarts, it overwrites the previously exported files, causing data loss?Wouldn't it be simpler to add the start time of the Collector as a suffix to the file name?

@OverOrion
Copy link
Contributor Author

Hey @atingchen,

Are you worried that when the Collector restarts, it overwrites the previously exported files, causing data loss?

Yes, that is the problem I've described.

Wouldn't it be simpler to add the start time of the Collector as a suffix to the file name?

I do not see how that could be simpler (received messages scattered across multiple files or just having one file), could you please elaborate?

@atingchen
Copy link
Contributor

This is indeed an issue that we should pay attention to and resolve. I have reviewed the code you submitted, and your solution is much simpler. @OverOrion.

@OverOrion
Copy link
Contributor Author

Thank you for your quick response @atingchen! Could you please allow the CI to run on the PR? (Not sure if you have the permission, just being hopeful that you do).

@atingchen
Copy link
Contributor

Thank you very much for your work @OverOrion~ I have provided you with some suggestions, and you are more than welcome to create a PR.

@OverOrion
Copy link
Contributor Author

Hey @atingchen,
Thanks for the quick reply, but I do not see any comments / review notes on the PR: #31369.
Can you please double check that you submitted your review?

@atingchen
Copy link
Contributor

Hi @OverOrion,
I have submitted my review.

This was referenced Feb 27, 2024
Closed
Open
djaglowski pushed a commit that referenced this issue Mar 6, 2024
@OverOrion
[exporter/file] add append mode (#31369)
75cfca8 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This adds a new option for configuring the append / truncate behavior of
the fileexporter.

**Link to tracking Issue:**
#31364

**Testing:** Added `TestAppend` unit test and manually tested using
`telemetrygen` and the following configuration:

```yaml
receivers:
  otlp:
    protocols:
      grpc:
        endpoint: 0.0.0.0:4317

exporters:
  file:
    path: ./receiver_output_append.log
    append: true
service:
  telemetry:
    metrics:
      level: detailed
      address: 0.0.0.0:9998
  pipelines:
    logs:
      receivers: [otlp]
      exporters: [file]
```

**Documentation:** <Describe the documentation added.>

TODO: 

- [x] add documentation once we reached agreement regarding
implementation / naming

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>
DougManton pushed a commit to DougManton/opentelemetry-collector-contrib that referenced this issue Mar 13, 2024
@OverOrion @DougManton
[exporter/file] add append mode (open-telemetry#31369)
2a2e45f 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This adds a new option for configuring the append / truncate behavior of
the fileexporter.

**Link to tracking Issue:**
open-telemetry#31364

**Testing:** Added `TestAppend` unit test and manually tested using
`telemetrygen` and the following configuration:

```yaml
receivers:
  otlp:
    protocols:
      grpc:
        endpoint: 0.0.0.0:4317

exporters:
  file:
    path: ./receiver_output_append.log
    append: true
service:
  telemetry:
    metrics:
      level: detailed
      address: 0.0.0.0:9998
  pipelines:
    logs:
      receivers: [otlp]
      exporters: [file]
```

**Documentation:** <Describe the documentation added.>

TODO: 

- [x] add documentation once we reached agreement regarding
implementation / naming

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this issue Mar 13, 2024
@OverOrion @XinRanZhAWS
[exporter/file] add append mode (open-telemetry#31369)
dd949ab 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This adds a new option for configuring the append / truncate behavior of
the fileexporter.

**Link to tracking Issue:**
open-telemetry#31364

**Testing:** Added `TestAppend` unit test and manually tested using
`telemetrygen` and the following configuration:

```yaml
receivers:
  otlp:
    protocols:
      grpc:
        endpoint: 0.0.0.0:4317

exporters:
  file:
    path: ./receiver_output_append.log
    append: true
service:
  telemetry:
    metrics:
      level: detailed
      address: 0.0.0.0:9998
  pipelines:
    logs:
      receivers: [otlp]
      exporters: [file]
```

**Documentation:** <Describe the documentation added.>

TODO: 

- [x] add documentation once we reached agreement regarding
implementation / naming

Signed-off-by: Szilard Parrag <szilard.parrag@axoflow.com>
@atoulme
Copy link
Contributor

atoulme commented Mar 26, 2024

Can this issue be closed now that #31369 is merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@OverOrion OverOrion

Labels
enhancement New feature or request exporter/file
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@atoulme @djaglowski @OverOrion @atingchen @andrzej-stencel