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

[extension/storage/filestorage] Document how to read files created by the file_storage extension #32180

Merged
merged 5 commits into from
Apr 16, 2024

Conversation

ycombinator
Copy link
Contributor

Description:

This PR adds documentation explaining how to read the contents of files created by the file_storage extension.

Link to tracking Issue:

@atoulme
Copy link
Contributor

atoulme commented Apr 5, 2024

I'm not sure about this. Are you trying to achieve a particular functionality or achieve a use case such as troubleshooting?

@atoulme
Copy link
Contributor

atoulme commented Apr 5, 2024

OK, I have read the associated issue. Would you mind framing this as a troubleshooting solution?

@atoulme atoulme added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 5, 2024

## Troubleshooting

When troubleshooting components that use file storage, it is sometimes helpful to read the raw contents of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a note that this strategy is subject to change.

The context here is that the underlying storage mechanism, aside from being a file, is not part of an API which we have ever guaranteed. Some suggestions have been made in the past to evaluate other key/value stores which may be more performant than the bbolt one we currently use. If we ever change, we would of course have to ensure migration is possible for some time, but ultimately we should be careful not to appear locked into the implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 4a32823b11.

Copy link
Contributor

@tiffany76 tiffany76 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from a documentation perspective.

Copy link
Contributor

@tiffany76 tiffany76 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small change to eliminate redundancy. Otherwise, LGTM.

Co-authored-by: Tiffany Hrabusa <30397949+tiffany76@users.noreply.github.com>
@djaglowski djaglowski merged commit 21a8eb1 into open-telemetry:main Apr 16, 2024
168 of 170 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 16, 2024
@ycombinator ycombinator deleted the doc-ext-filestorage branch April 22, 2024 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension/storage Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants