-
Notifications
You must be signed in to change notification settings - Fork 21
feat: enable Logs Drilldown link in Metrics Drilldown #1389
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
|
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.
Is this something metrics will want released to prod quickly?
targets: [ | ||
PluginExtensionPoints.DashboardPanelMenu, | ||
PluginExtensionPoints.ExploreToolbarAction, | ||
'grafana-metricsdrilldown-app/open-in-logs-drilldown/v1', |
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.
If you update with latest main. I think the only change you will need to make is adding 'grafana-metricsdrilldown-app/open-in-logs-drilldown/v1',
src/services/extensions/links.ts
Outdated
@@ -35,32 +35,18 @@ export const ExtensionPoints = { | |||
MetricInvestigation: 'grafana-lokiexplore-app/investigation/v1', | |||
} as const; | |||
|
|||
/* eslint-disable sort/object-properties */ |
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.
This deleted rule made the diff very hard to understand, so it'd be amazing if you can revert these changes after syncing with main 🙏
94ab6c1
to
9121160
Compare
// `plugin.addLink` requires these types; unfortunately, the correct `PluginExtensionAddedLinkConfig` type is not exported with 11.2.x | ||
// TODO: fix this type when we move to `@grafana/data` 11.3.x |
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.
No longer needed, thanks to 65e6e53#diff-f2ed28ac7a0702c63eb0a49c08ffbc641e45c97f1019c853a0e5e09a975d133e
I rebased onto the latest |
@NWRichmond can you add |
@NWRichmond also you'll need to resign the CLA, see the comment above, we recycled tokens and now everyone has to re-sign |
Thank you! Signed ✅ |
Context
This PR supports grafana/metrics-drilldown#524
What does this do?
Adds
'grafana-metricsdrilldown-app/open-in-logs-drilldown/v1'
as another extension point that can leverage the "Open in Grafana Logs Drilldown" linkDemo
On a feature branch in Metrics Drilldown, the extension point is working as expected in the Related Logs tab:
demo.related.logs.open.in.related.logs.preserve.time.range.and.labels.mov
Testing the full experience
It's tedious to do all of the setup involved in testing that this extension point not only exists, but also achieves the goals described in grafana/metrics-drilldown#524. But if you're feeling brave, the full testing instructions are provided in the How to test section of grafana/metrics-drilldown#535.