fix: add back gzip support for grpc otlp endpoint #11434
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
kruskall
added
backport-8.9
axw
approved these changes
Aug 21, 2023
mergify bot
pushed a commit
that referenced
this pull request
Aug 21, 2023
* fix: add back gzip support for grpc otlp endpoint * changelog: add changelog entry (cherry picked from commit cfce80b) # Conflicts: # changelogs/head.asciidoc
mergify bot
pushed a commit
that referenced
this pull request
Aug 21, 2023
* fix: add back gzip support for grpc otlp endpoint * changelog: add changelog entry (cherry picked from commit cfce80b) # Conflicts: # changelogs/head.asciidoc
lahsivjar
pushed a commit
that referenced
this pull request
Aug 22, 2023
* fix: add back gzip support for grpc otlp endpoint * changelog: add changelog entry (cherry picked from commit cfce80b) # Conflicts: # changelogs/head.asciidoc
lahsivjar
pushed a commit
that referenced
this pull request
Aug 22, 2023
…1434) (#11436) (cherry picked from commit cfce80b) Co-authored-by: kruskall <[email protected]>
mergify bot
added a commit
that referenced
this pull request
Aug 22, 2023
) (#11435) * fix: add back gzip support for grpc otlp endpoint (#11434) * fix: add back gzip support for grpc otlp endpoint * changelog: add changelog entry (cherry picked from commit cfce80b) # Conflicts: # changelogs/head.asciidoc * changelog: delete head changelog --------- Co-authored-by: kruskall <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
dmathieu
reviewed
Aug 23, 2023
| @@ -206,7 +206,7 @@ func TestOTLPGRPCMetrics(t *testing.T) { | |||
|
|
|||
| // opentelemetry-go does not support sending Summary metrics, | |||
| // so we send them using the lower level OTLP/gRPC client. | |||
| conn, err := grpc.Dial(serverAddr(srv), grpc.WithInsecure(), grpc.WithBlock()) | |||
| conn, err := grpc.Dial(serverAddr(srv), grpc.WithInsecure(), grpc.WithBlock(), grpc.WithDefaultCallOptions(grpc.UseCompressor("gzip"))) | |||
There was a problem hiding this comment.
Could there be a future regression where it's requests without gzip that don't work? Should we also test without gzip enabled?
There was a problem hiding this comment.
That's unlikely, other tests are sending uncompressed requests to the grpc endpoint so they would fail
|
Verified with 8.10.0-BC1 using elastic/apm-tools#22 |
bmorelli25
pushed a commit
to bmorelli25/apm-server
that referenced
this pull request
Sep 5, 2023
* fix: add back gzip support for grpc otlp endpoint * changelog: add changelog entry
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.
Motivation/summary
gzip support was removed in #11072 along with UP code.
_ "google.golang.org/grpc/encoding/gzip"was imported inx-pack/apm-server/profiling/collector.goin theprofilingpackage which was imported into the main package, enabling gzip support.This PR adds back the import which runs the
initfunction enabling gzip support.The grpc otlp tests have been updated to send gzip request to validate that the feature is working.
The fix is backported to 8.10 and 8.9 and it's related to SDH
#1053Checklist
apmpackagehave been made)For functional changes, consider:
How to test these changes
Related issues