Skip to content

fix(loki sink): update to use the global list of compression algorithms#19104

Merged
StephenWakely merged 3 commits intoopw/releases/v1.7from
stephen/loki_compression2
Nov 9, 2023
Merged

fix(loki sink): update to use the global list of compression algorithms#19104
StephenWakely merged 3 commits intoopw/releases/v1.7from
stephen/loki_compression2

Conversation

@StephenWakely
Copy link
Contributor

The Loki sink had some special code to handle snappy. snappy was #18676 to the global list of compression schemes. This meant Loki was specifying snappy twice. It looks like this broke the schema validation.

This PR removes the special config option for snappy, but hopefully still handles the choice of snappy differently (encodes it as protobuf), as it did before.

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
@StephenWakely StephenWakely requested review from a team as code owners November 9, 2023 17:29
@StephenWakely StephenWakely requested a review from a team November 9, 2023 17:29
@github-actions github-actions bot added domain: sinks Anything related to the Vector's sinks domain: external docs Anything related to Vector's external, public documentation labels Nov 9, 2023
@datadog-vectordotdev
Copy link

datadog-vectordotdev bot commented Nov 9, 2023

Datadog Report

Branch report: stephen/loki_compression2
Commit report: 7227fd6

vector: 0 Failed, 0 New Flaky, 2017 Passed, 0 Skipped, 1m 21.03s Wall Time

Comment on lines +94 to 100
if self.compression == Compression::Snappy {
// Snappy compression is applied after converting the batch to protobuf so
// we need to handle this separately.
Compression::None
} else {
self.compression
}
Copy link
Contributor

@dsmith3197 dsmith3197 Nov 9, 2023

Choose a reason for hiding this comment

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

Does it make more sense to remove the snap encoding here and defer to the request builder here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm.. potentially. I'll look into that for merging into master.

@StephenWakely StephenWakely merged commit f9bd6bb into opw/releases/v1.7 Nov 9, 2023
@StephenWakely StephenWakely deleted the stephen/loki_compression2 branch November 9, 2023 19:58
@jszwedko jszwedko restored the stephen/loki_compression2 branch November 15, 2023 20:24
neuronull pushed a commit that referenced this pull request Nov 16, 2023
…ms (#19104)

* Update to use the global list of compression algorithms

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>

* Mention snappy compression using protocol buffers

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>

* Snappy is compressed separately

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>

---------

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
neuronull added a commit that referenced this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: external docs Anything related to Vector's external, public documentation domain: sinks Anything related to the Vector's sinks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants