[exporter/clickhouse] Add compress option to config, enabled by default
#34365
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
Frapschen
reviewed
Aug 6, 2024
Frapschen
approved these changes
Aug 6, 2024
mx-psi
reviewed
Aug 7, 2024
hanjm
approved these changes
Aug 7, 2024
…emetry-collector-contrib into compression-config
|
Resolved all review notes. Thanks! |
Frapschen
approved these changes
Aug 8, 2024
mx-psi
approved these changes
Aug 8, 2024
f7o
pushed a commit
to f7o/opentelemetry-collector-contrib
that referenced
this pull request
Sep 12, 2024
…ault (open-telemetry#34365) **Description:** This change adds a new `compress` option to the config and sets it to `lz4` by default. In the current version of the exporter, users must know to provide `compress` in the DSN URL to gain the network performance benefits of compression. The only way they would have known this before is if they copied the sample from the README, but this is likely replaced when they paste their server address. ClickHouse has excellent compression for storage and network. It is recommended to enable it for clients such as the OTel exporter to improve performance. In summary: - Added `compress` field to config - `endpoint` (DSN URL) and `connection_params` takes priority - If left empty from all sources, will default to `lz4` - Valid options are based on the underlying `clickhouse-go` driver: `none` (disabled), `zstd`, `lz4` (default), `gzip`, `deflate`, `br`, `true` (lz4). The `true` option comes from an older version of `clickhouse-go` and is an alias for `lz4`. To prevent unexpected changes in behavior, I have manually re-added this check to the config parser instead of assuming the driver will still interpret it as `lz4`. **Testing:** - Updated unit tests for DSN + config parsing - Ran integration tests locally **Documentation:** - Updated README config options list + sample - Added changelog --------- Co-authored-by: Pablo Baeyens <[email protected]>
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.
Description:
This change adds a new
compressoption to the config and sets it tolz4by default.In the current version of the exporter, users must know to provide
compressin the DSN URL to gain the network performance benefits of compression. The only way they would have known this before is if they copied the sample from the README, but this is likely replaced when they paste their server address.ClickHouse has excellent compression for storage and network. It is recommended to enable it for clients such as the OTel exporter to improve performance.
In summary:
compressfield to configendpoint(DSN URL) andconnection_paramstakes prioritylz4clickhouse-godriver:none(disabled),zstd,lz4(default),gzip,deflate,br,true(lz4).The
trueoption comes from an older version ofclickhouse-goand is an alias forlz4. To prevent unexpected changes in behavior, I have manually re-added this check to the config parser instead of assuming the driver will still interpret it aslz4.Testing:
Documentation: