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

[exporter/file] #14690

Closed
djaglowski opened this issue Oct 3, 2022 · 6 comments
Closed

[exporter/file] #14690

djaglowski opened this issue Oct 3, 2022 · 6 comments
Assignees
Labels
bug Something isn't working priority:p2 Medium

Comments

@djaglowski
Copy link
Member

What happened?

Description

rotation settings were added in v0.60.0. Unfortunately, they are enabled by default, which was an undeclared breaking change.

I'm opening this issue to discuss whether the previous functionality should be restored.

Steps to Reproduce

Configure the file exporter without specifying rotation settings.

Expected Result

File rotation is not enabled.

Actual Result

File rotation is enabled.

Collector version

v0.60.0

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

The intended design would have file rotation enabled only if the user specified at least rotation: in the config. However, if specified, related default settings would apply. This would allow users to enable rotation very easily without having to specify specific settings.

Since the behavior has already changed, the question should be asked - should it be corrected in the next version, to behave as described above? Alternately, should it stay ie. is there a strong case to be made that rotation should be enabled by default?

@djaglowski djaglowski added bug Something isn't working needs triage New item requiring triage priority:p2 Medium and removed needs triage New item requiring triage labels Oct 3, 2022
@djaglowski
Copy link
Member Author

@atingchen, I'm curious your thoughts on this.

@atingchen
Copy link
Contributor

@djaglowski Sorry for causing this breaking change.
When the amount of data exported by the user is small or only some data is exported each time, I think the performance of the rotation exporting a 100MB file by default is the same as before.
When the amount of data exported from user data is large or grows infinitely, the following problems may occur:

  • Larger files are harder to manipulate.
  • File systems run out of space.

For these reasons, I set rotation as the default.
In order to avoid problems that may be caused to users by the infinite growth of files, should we set rotation as default and provide a flag to disable it?

@djaglowski
Copy link
Member Author

I am in favor of having rotation disabled by default. However, if possible, we should find a way to do this that does not require a flag. My ideal would be that it behaves like this:

file/no_rotation:
  path: ./foo

file/rotation_with_default_settings:
  path: ./foo
  rotation:

file/rotation_with_custom_settings:
  path: ./foo
  rotation:
    max_megabytes: 1234
    ...

@atingchen
Copy link
Contributor

I'll open a PR to fix this.

@djaglowski
Copy link
Member Author

Thanks @atingchen

@evan-bradley
Copy link
Contributor

Closing as resolved via #14705.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p2 Medium
Projects
None yet
Development

No branches or pull requests

3 participants