-
Notifications
You must be signed in to change notification settings - Fork 981
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
Complete prometheus manager wiring/modeling #9550
Complete prometheus manager wiring/modeling #9550
Conversation
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
Signed-off-by: daniel <51932404+d-buckner@users.noreply.github.com>
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
Co-authored-by: Joshua Li <joshuali925@gmail.com> Signed-off-by: daniel <51932404+d-buckner@users.noreply.github.com>
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
Signed-off-by: daniel <51932404+d-buckner@users.noreply.github.com>
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
async (context, request, response) => { | ||
const { dataConnectionId, dataConnectionType, resourceType, resourceName } = request.params; | ||
if (dataConnectionType === 'prometheus') { | ||
const resources = await new PrometheusManager().getResources(context, request, { |
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.
@joshuali925 I know you mentioned this could be added to the route handler context. Can you share where that's defined? Happy to use a shared instance in context in follow up.
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.
you can define route context like this
OpenSearch-Dashboards/src/plugins/query_enhancements/server/plugin.ts
Lines 78 to 85 in e8cf0ca
core.http.registerRouteHandlerContext('query_assist', () => ({ | |
logger: this.logger, | |
configPromise: this.initializerContext.config | |
.create<ConfigSchema>() | |
.pipe(first()) | |
.toPromise(), | |
dataSourceEnabled: !!dataSource, | |
})); |
then it's accessible in the route handler's context.<context-name>
variable
It actually must be a shared instance, because the overridden client from other plugins won't work if every time the manager is newly created. might be better if you can make it a singleton
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.
Yeah, that's what I was worried about as well. Will export a singleton, register in this context, and then consume from handler context. I'll send out a commit shortly to address.
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've updated the prometheus manager so it's only vended as a singleton. I didn't get a chance to update the router context, but will include in next change.
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
595b92c
to
7412982
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/prometheus #9550 +/- ##
=====================================================
Coverage ? 61.72%
=====================================================
Files ? 3822
Lines ? 91971
Branches ? 14578
=====================================================
Hits ? 56771
Misses ? 31542
Partials ? 3658
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
828a4f7
to
a2d9aa2
Compare
Signed-off-by: Daniel Rowe <rowdane@amazon.com>
a2d9aa2
to
8b32bd0
Compare
33115c9
into
opensearch-project:feature/prometheus
Description
This is to update the prometheus manager to use the new resource API routes and better model the query and response types
Changelog
Check List
yarn test:jest
yarn test:jest_integration