Add zstd decompression support to HTTPServerSettings #7927
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tigrannajaryan
force-pushed
the
feature/tigran/addzstd
branch
from
0f4aea6 to
2020d10
Compare
tigrannajaryan
changed the title
tigrannajaryan
force-pushed
the
feature/tigran/addzstd
branch
from
2020d10 to
ba4ea24
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #7927 +/- ##
==========================================
- Coverage 91.05% 91.05% -0.01%
==========================================
Files 298 299 +1
Lines 14912 14944 +32
==========================================
+ Hits 13578 13607 +29
- Misses 1057 1059 +2
- Partials 277 278 +1
☔ View full report in Codecov by Sentry. |
tigrannajaryan
force-pushed
the
feature/tigran/addzstd
branch
from
ba4ea24 to
2e64b76
Compare
atoulme
approved these changes
Jun 20, 2023
codeboten
reviewed
Jun 20, 2023
tigrannajaryan
force-pushed
the
feature/tigran/addzstd
branch
from
2e64b76 to
a80df15
Compare
This adds ability to decompress zstd-compressed HTTP requests to all receivers that use HTTPServerSettings. Also added missing error handling for the case when an unsupported compression type was used in the request. Now it correctly returns 400 Bad Request. Also added a unit test to verify this case. Once this is merged I will submit a PR in contrib repo to add end-to-end tests that use zstd compression in the testbed.
tigrannajaryan
force-pushed
the
feature/tigran/addzstd
branch
from
a80df15 to
f901fda
Compare
Done. |
|
Duplicate of #7636? |
|
Please look at the other PR and incorporate that change as well. |
|
I looked at #7636 and it seems to do 2 additional things:
|
codeboten
approved these changes
Jun 22, 2023
codeboten
pushed a commit
that referenced
this pull request
Jun 29, 2023
This provides the ability to add support for compression types which are not supported in core, to be supported by individual components. This change addresses an issue caused by #7927, which prevents receivers that use confighttp from adding support for their content-encoding types easily. They still can by implementing an error handler, but that seems messy. An alternative to this PR is to not return an error for an unsupported encoding type as was the case before #7927 If the approach is acceptable, I will add the unit tests and changelog Signed-off-by: Alex Boten <aboten@lightstep.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds ability to decompress zstd-compressed HTTP requests to all receivers that use HTTPServerSettings.
Also added missing error handling for the case when an unsupported compression type was used in the request. Now it correctly returns 400 Bad Request. Also added a unit test to verify this case.
Once this is merged I will submit a PR in contrib repo to add end-to-end tests that use zstd compression in the testbed.