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

File storage extension - invalid file name characters must be encoded #3148

Closed
djaglowski opened this issue Apr 16, 2021 · 6 comments · Fixed by #20896
Closed

File storage extension - invalid file name characters must be encoded #3148

djaglowski opened this issue Apr 16, 2021 · 6 comments · Fixed by #20896
Assignees
@andrzej-stencel
Labels
extension/storage/filestorage

Comments

@djaglowski
Copy link
Member

djaglowski commented Apr 16, 2021 •
edited
Loading

The invalid characters need to be encoded using valid ones. I guess something like urlencode/QueryEscape (https://golang.org/pkg/net/url/#QueryEscape) should work. You will need to check that it escape the correct set of characters (it needs to be a superset of what we need). I am not aware of a precise function that does this for path names.

Originally posted by @tigrannajaryan in #3087 (comment)
@djaglowski djaglowski self-assigned this Apr 16, 2021
@djaglowski djaglowski mentioned this issue Apr 16, 2021
Merged
@tigrannajaryan tigrannajaryan added this to the Basic Logs Support milestone Apr 16, 2021
@tigrannajaryan tigrannajaryan added help wanted Extra attention is needed spec:logs labels Apr 16, 2021
@djaglowski djaglowski removed this from the Basic Logs Support milestone Sep 16, 2021
@djaglowski djaglowski removed their assignment Nov 12, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Nov 7, 2022
@github-actions
Copy link
Contributor

Pinging code owners for extension/storage/filestorage: @djaglowski. See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot removed the Stale label Mar 19, 2023
@djaglowski djaglowski mentioned this issue Apr 7, 2023
Closed
@andrzej-stencel
Copy link
Member

andrzej-stencel commented Apr 13, 2023 •
edited
Loading

The https://golang.org/pkg/net/url/#QueryEscape function specifically is not a good fit, as it puts percent characters % in the output. The percent characters % have a special meaning in Windows, as environment variables - e.g. %PATH%.

andrzej-stencel added a commit to andrzej-stencel/opentelemetry-collector-contrib that referenced this issue Apr 13, 2023
@andrzej-stencel
[extension/filestorage] replace slashes in component names
c025ee1 
Fixes open-telemetry#20731

Note that it does not fix open-telemetry#3148, which is broader in scope.
We should ideally replace not only a forward slash,
but all characters that can be problematic on various platforms.
andrzej-stencel added a commit to andrzej-stencel/opentelemetry-collector-contrib that referenced this issue Apr 13, 2023
@andrzej-stencel
[extension/filestorage] replace slashes in component names
ef4a834 
Fixes open-telemetry#20731

Note that it does not fix open-telemetry#3148, which is broader in scope.
We should ideally replace not only a forward slash,
but all characters that can be problematic on various platforms.
andrzej-stencel added a commit to andrzej-stencel/opentelemetry-collector-contrib that referenced this issue Apr 13, 2023
@andrzej-stencel
[extension/filestorage] replace slashes in component names
ead3989 
Fixes open-telemetry#20731

Note that it does not fix open-telemetry#3148, which is broader in scope.
We should ideally replace not only a forward slash,
but all characters that can be problematic on various platforms.
@andrzej-stencel andrzej-stencel mentioned this issue Apr 13, 2023
Merged
@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@github-actions github-actions bot added the Stale label Jun 13, 2023
andrzej-stencel added a commit to andrzej-stencel/opentelemetry-collector-contrib that referenced this issue Jun 28, 2023
@andrzej-stencel
[extension/filestorage] replace path-unsafe characters in component n…
7a877b5 
…ames

Fixes open-telemetry#3148
andrzej-stencel added a commit to andrzej-stencel/opentelemetry-collector-contrib that referenced this issue Jun 28, 2023
@andrzej-stencel
[extension/filestorage] replace path-unsafe characters in component n…
b916f7a 
…ames

Fixes open-telemetry#3148
@andrzej-stencel
Copy link
Member

andrzej-stencel commented Jun 29, 2023 •
edited
Loading

Here's the PR to fix this issue:

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@github-actions github-actions bot added the Stale label Aug 29, 2023
@djaglowski djaglowski removed the Stale label Aug 29, 2023
@andrzej-stencel andrzej-stencel self-assigned this Sep 11, 2023
@andrzej-stencel andrzej-stencel removed the help wanted Extra attention is needed label Sep 11, 2023
djaglowski pushed a commit that referenced this issue Oct 3, 2023
@andrzej-stencel
[extension/filestorage] replace path-unsafe characters in component n…
91e56e4 
…ames (#20896)

Fixes #3148

This change was initially created to only fix #20731 by replacing the
slash `/` characters in component names with a tilde `~`. After that, we
decided that it is a breaking change, and so it's better to fix other
characters as well in a single breaking change.
sumo-drosiek pushed a commit to sumo-drosiek/opentelemetry-collector-contrib that referenced this issue Oct 4, 2023
@andrzej-stencel @sumo-drosiek
[extension/filestorage] replace path-unsafe characters in component n…
02a2544 
…ames (open-telemetry#20896)

Fixes open-telemetry#3148

This change was initially created to only fix open-telemetry#20731 by replacing the
slash `/` characters in component names with a tilde `~`. After that, we
decided that it is a breaking change, and so it's better to fix other
characters as well in a single breaking change.
jorgeancal pushed a commit to jorgeancal/opentelemetry-collector-contrib that referenced this issue Oct 9, 2023
@andrzej-stencel @jorgeancal
[extension/filestorage] replace path-unsafe characters in component n…
f1cbbae 
…ames (open-telemetry#20896)

Fixes open-telemetry#3148

This change was initially created to only fix open-telemetry#20731 by replacing the
slash `/` characters in component names with a tilde `~`. After that, we
decided that it is a breaking change, and so it's better to fix other
characters as well in a single breaking change.
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this issue Nov 12, 2023
@andrzej-stencel @jmsnll
[extension/filestorage] replace path-unsafe characters in component n…
d02accd 
…ames (open-telemetry#20896)

Fixes open-telemetry#3148

This change was initially created to only fix open-telemetry#20731 by replacing the
slash `/` characters in component names with a tilde `~`. After that, we
decided that it is a breaking change, and so it's better to fix other
characters as well in a single breaking change.
@andrzej-stencel andrzej-stencel mentioned this issue Jan 24, 2024
Merged
djaglowski pushed a commit that referenced this issue Jan 24, 2024
@andrzej-stencel
[extension/filestorage] enable feature gate `extension.filestorage.re…
d316a95 
…placeUnsafeCharacters` (#30755)

**Description:**

Moves the feature gate `extension.filestorage.replaceUnsafeCharacters`
to Beta stage, which means it is now enabled by default. This is
according to the schedule for this feature gate described in the File
Storage extension's
[README](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.92.0/extension/storage/filestorage/README.md#extensionfilestoragereplaceunsafecharacters).

**Link to tracking Issue:**

-
#3148

**Testing:**

Updated tests to not require enabling of the feature gate.

**Documentation:**

Updated version in which feature gate was moved to Beta.
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this issue Feb 1, 2024
@andrzej-stencel @cparkins
[extension/filestorage] enable feature gate `extension.filestorage.re…
2a8d0f8 
…placeUnsafeCharacters` (open-telemetry#30755)

**Description:**

Moves the feature gate `extension.filestorage.replaceUnsafeCharacters`
to Beta stage, which means it is now enabled by default. This is
according to the schedule for this feature gate described in the File
Storage extension's
[README](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.92.0/extension/storage/filestorage/README.md#extensionfilestoragereplaceunsafecharacters).

**Link to tracking Issue:**

-
open-telemetry#3148

**Testing:**

Updated tests to not require enabling of the feature gate.

**Documentation:**

Updated version in which feature gate was moved to Beta.
anthoai97 pushed a commit to anthoai97/opentelemetry-collector-contrib that referenced this issue Feb 12, 2024
@andrzej-stencel @anthoai97
[extension/filestorage] enable feature gate `extension.filestorage.re…
f8cf98b 
…placeUnsafeCharacters` (open-telemetry#30755)

**Description:**

Moves the feature gate `extension.filestorage.replaceUnsafeCharacters`
to Beta stage, which means it is now enabled by default. This is
according to the schedule for this feature gate described in the File
Storage extension's
[README](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.92.0/extension/storage/filestorage/README.md#extensionfilestoragereplaceunsafecharacters).

**Link to tracking Issue:**

-
open-telemetry#3148

**Testing:**

Updated tests to not require enabling of the feature gate.

**Documentation:**

Updated version in which feature gate was moved to Beta.
@andrzej-stencel andrzej-stencel mentioned this issue Apr 15, 2024
Merged
andrzej-stencel added a commit that referenced this issue Apr 16, 2024
@andrzej-stencel
[extension/filestorage] move feature gate to stable (#32374)
8a259dd 
**Description:**

Moves the feature gate `extension.filestorage.replaceUnsafeCharacters`
to Stable stage, which means it cannot be disabled. This is according to
the schedule for this feature gate described in the File Storage
extension's
[README](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.97.0/extension/storage/filestorage/README.md#extensionfilestoragereplaceunsafecharacters).

**Link to tracking Issue:**

-
#3148

**Testing:**

No changes.

**Documentation:**

Updated version in which feature gate was moved to Beta.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@andrzej-stencel @rimitchell
[extension/filestorage] move feature gate to stable (open-telemetry#3…
9889c3e 
…2374)

**Description:**

Moves the feature gate `extension.filestorage.replaceUnsafeCharacters`
to Stable stage, which means it cannot be disabled. This is according to
the schedule for this feature gate described in the File Storage
extension's
[README](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.97.0/extension/storage/filestorage/README.md#extensionfilestoragereplaceunsafecharacters).

**Link to tracking Issue:**

-
open-telemetry#3148

**Testing:**

No changes.

**Documentation:**

Updated version in which feature gate was moved to Beta.
@andrzej-stencel andrzej-stencel mentioned this issue May 27, 2024
Merged
djaglowski pushed a commit that referenced this issue May 31, 2024
@andrzej-stencel
[extension/filestorage] remove feature flag `extension.filestorage.re…
2b1c4a6 
…placeUnsafeCharacters` (#33236)

**Description:**

Removes the feature flag according to schedule.

**Link to tracking Issue:**

-
#3148

**Testing:**

No changes.

**Documentation:**

Replaced the description of the feature gate with the description of the
feature.
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this issue Jul 11, 2024
@andrzej-stencel @cparkins
[extension/filestorage] remove feature flag `extension.filestorage.re…
325e16b 
…placeUnsafeCharacters` (open-telemetry#33236)

**Description:**

Removes the feature flag according to schedule.

**Link to tracking Issue:**

-
open-telemetry#3148

**Testing:**

No changes.

**Documentation:**

Replaced the description of the feature gate with the description of the
feature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andrzej-stencel andrzej-stencel

Labels
extension/storage/filestorage
Projects
None yet
Milestone
No milestone
4 participants
@atoulme @tigrannajaryan @djaglowski @andrzej-stencel