-
Notifications
You must be signed in to change notification settings - Fork 281
Adding Subscription Metrics to Metric Recorder and Onboarding M365 to Auth Metrics from Metrics Recorder #6363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
aabd6e8 to
8af181c
Compare
86ec81f to
b6c0d6e
Compare
| // Start subscriptions for each content type | ||
| headers.setContentLength(0); | ||
| // Start subscriptions for each content type | ||
| headers.setContentLength(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation was previously off, this fixes the incorrect indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in the future you can run this command to fix indentation
./gradlew :data-prepper-plugins:saas-source-plugins:microsoft-office365-source:spotlessApply
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you for this command. Just ran it and looks like no changes were made so the new indentation is correct!
| * NOTE: Any new Office 365 specific metrics should be implemented here to maintain proper | ||
| * separation of vendor-specific functionality and keep Office 365 metrics centralized. | ||
| */ | ||
| public class Office365MetricsRecorder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be hard to maintain metric allowlist if we start to introduce per service metric.
We should try to make this metric generic so can be extended for similar cases in the future.
I would suggest add StartSubscription recorder to the vendorAPIMetric instead of having its own one.
1e0c7ba to
c2324ae
Compare
| // Start subscriptions for each content type | ||
| headers.setContentLength(0); | ||
| // Start subscriptions for each content type | ||
| headers.setContentLength(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in the future you can run this command to fix indentation
./gradlew :data-prepper-plugins:saas-source-plugins:microsoft-office365-source:spotlessApply
c2324ae to
d9ef33f
Compare
d9ef33f to
5527039
Compare
… Auth Metrics from Metrics Recorder Signed-off-by: Alexander Christensen <[email protected]>
5527039 to
ad4bb22
Compare
… Auth Metrics from Metrics Recorder (opensearch-project#6363) Signed-off-by: Alexander Christensen <[email protected]> Signed-off-by: Nathan Wand <[email protected]>
Description
This PR introduces comprehensive metrics tracking for Microsoft Office 365 API operations in the Data Prepper M365 source plugin by extending the VendorAPIMetricsRecorder to support both start subscription APIs and authentication APIs, maintaining centralized vendor API metrics while adding M365-specific operation tracking.
Changes
New Metrics in VendorAPIMetricsRecorder
Modified Components
Test Coverage
Metrics Added
Testing:
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.