[exporter/elasticsearch] Use confighttp.ClientConfig #33367
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
Move Authentication out of ClientConfig, and replace ClientConfig with confighttp.ClientConfig. Implement StartFunc, and create bulk indexer there, so we have access to component.Host for calling confighttp.ClientConfig.ToClient. This enables all standard confighttp functionality, such as auth extensions.
axw
force-pushed
the
elasticsearchexporter-confighttp
branch
from
511fead
to
d34e708
Compare
carsonip
reviewed
Jun 6, 2024
Co-authored-by: Carson Ip <carsonip@users.noreply.github.com>
@carsonip thanks - the PR is ready for review. |
codefromthecrypt
approved these changes
Jun 7, 2024
There was a problem hiding this comment.
lookin sharp! added some tidies to consider or dismiss, nothing important.
should we add a test for the round-robin? add a bad host like https://not-a-real-host.com:9200 in front of a working one (setting timeouts to a small value so tests don't take too long)?
Co-authored-by: Adrian Cole <64215+codefromthecrypt@users.noreply.github.com>
Co-authored-by: Andrzej Stencel <andrzej.stencel@elastic.co>
andrzej-stencel
approved these changes
Jun 12, 2024
carsonip
approved these changes
Jun 12, 2024
andrzej-stencel
merged commit 54d0dcb
into
open-telemetry:main
158 of 162 checks passed
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:
Move
Authenticationout ofClientConfig, and replaceClientConfigwithconfighttp.ClientConfig. This enables all commonconfighttpfunctionality, such as auth extensions, with the exception of "compression".For now compression remains unconfigurable, and is always enabled and uses gzip (this is defined within the go-docappender dependency). We should add some benchmarks (#32504) before switching to the
confighttp-based compressors.The exporter now implements a
component.StartFunc, and we create the bulk indexer there so that we have access tocomponent.Hostfor callingconfighttp.ClientConfig.ToClient.Link to tracking Issue:
N/A
Testing:
confighttp'sauthis supported, with a mock client authbasicauthextension, verifying the username/password are passed through to ES as configuredDocumentation:
Updated the exporter's README:
confighttp'sendpointsetting is now supported tooconfighttp,configtls, andsender_queuesettings, and replaced with links to their docs