AWS: Add MetricPublisher configuration support for S3FileIO (#15182)#16235
Open
yadavay-amzn wants to merge 2 commits intoapache:mainfrom
Open
AWS: Add MetricPublisher configuration support for S3FileIO (#15182)#16235yadavay-amzn wants to merge 2 commits intoapache:mainfrom
yadavay-amzn wants to merge 2 commits intoapache:mainfrom
Conversation
…5182) Adds support for configuring a custom AWS SDK MetricPublisher for S3FileIO via the s3.metrics-publisher-impl catalog property. This enables observability into S3 SDK-level metrics (latency, retries, errors) for debugging and monitoring. - S3FileIOProperties: adds METRICS_PUBLISHER_IMPL property and the applyMetricsPublisherConfiguration() method. Loads the publisher via a static create(Map) factory when present, or a no-arg constructor otherwise. - DefaultS3FileIOAwsClientFactory: applies the metrics publisher configuration to both the sync s3() and async s3Async() client builders. - Tests cover factory-method, no-arg constructor, disabled, and invalid-class paths. This PR builds on the design and initial implementation from apache#15122 by @rcjverhoef (PR stale-closed). Review feedback from @geruh on that PR (apply to S3 async client, clean imports, invalid-class test) is addressed here. Closes apache#15182 Co-authored-by: rocco.verhoef <rcjverhoef@gmail.com>
Split the create(Map) factory lookup from its invocation so the error message correctly identifies whether the factory method was missing or present-but-threw. Previously, a create(Map) method that existed and threw would be misreported as 'Cannot create MetricPublisher from class X' with the factory exception as the cause, giving users the false impression they needed to change the method signature when the actual issue was a configuration error inside the factory. The new error message states whether the failure occurred via create(Map) or via the no-arg constructor, so users can diagnose the right path without misreading the stack trace. Add regression tests asserting the distinction (via create(Map) vs via no-arg constructor) so the behaviour cannot silently regress.
yadavay-amzn
force-pushed
the
fix/15182-metrics-publisher
branch
from
fdd699f to
d5a9856
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Adds support for configuring a custom AWS SDK
MetricPublisherfor S3FileIO via thes3.metrics-publisher-implcatalog property. This enables observability into S3 SDK-level metrics (latency, retries, errors) for debugging and monitoring.This PR builds on the design and initial implementation from #15122 by @rcjverhoef (PR stale-closed). The review feedback from @geruh on that PR — apply to the S3 async client, clean up fully-qualified imports, and add a test for invalid/missing classes — is addressed here.
Commits in this PR
AWS: Add MetricPublisher configuration support for S3FileIO— co-authored with @rcjverhoef. AddsMETRICS_PUBLISHER_IMPLproperty, theapplyMetricsPublisherConfigurationmethod, wires it into both the syncs3()and asyncs3Async()paths, adds happy-path tests and docs.AWS: Improve MetricPublisher factory error reporting— splits factory lookup from factory invocation so the error message correctly identifies whethercreate(Map)was missing or present-but-threw. Adds regression tests for both throw paths.Changes
METRICS_PUBLISHER_IMPLproperty,applyMetricsPublisherConfiguration()method that loads the publisher via a staticcreate(Map)factory or, as a fallback, a no-arg constructor.aws.md.Usage
The class must implement
software.amazon.awssdk.metrics.MetricPublisherand provide either a staticcreate(Map<String, String>)factory method or a no-arg constructor.Closes #15182