ENH: metrics in secret extension #3922
Conversation
Signed-off-by: George Chen <[email protected]>
Signed-off-by: George Chen <[email protected]>
chenqi0805
requested review from
engechas,
graytaylor0,
dinujoh,
kkondaka,
asifsmohammed,
dlvenable and
oeyh
as code owners
| @@ -33,6 +35,7 @@ public AwsSecretPlugin(final AwsSecretPluginConfig awsSecretPluginConfig) { | |||
| this.pluginConfigPublisher = new AwsSecretsPluginConfigPublisher(); | |||
| pluginConfigValueTranslator = new AwsSecretsPluginConfigValueTranslator(secretsSupplier); | |||
| scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(); | |||
| pluginMetrics = PluginMetrics.fromNames("secrets", "aws"); | |||
There was a problem hiding this comment.
I tend to think that this should be fromNames("aws", "secrets")
I'm not sure that we will even have other secrets, so this seems to create an unnecessary extra layer.
There was a problem hiding this comment.
This is to be consistent with
There was a problem hiding this comment.
The documentation indicates that we go from a componentId to a componentScope.
I think the component in this case is the aws-plugin. So I would be in favor of using aws as the componentId.
@asifsmohammed , Any thoughts from you end?
There was a problem hiding this comment.
I think what George has in the PR is right order. We do <componentScope> + "." + <componentId>
| try { | ||
| secretsSupplier.refresh(secretConfigId); | ||
| secretsRefreshSuccessCounter.increment(); | ||
| pluginConfigPublisher.notifyAllPluginConfigObservable(); |
There was a problem hiding this comment.
Question: I am not sure how long this operations is going to take, can it increase the timer metric which tracks refresh duration?
There was a problem hiding this comment.
Yes. I include it in the timer to track the total time. But this operation should all be in-memory and I expect it to be much less time consuming compared to get secret call
Description
This PR adds metrics on AWS secrets extension plugin
Issues Resolved
Resolves #[Issue number to be closed when this PR is merged]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.