Skip to content
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

New data sources for widgets #304

Merged

Conversation

roughbits01
Copy link
Contributor

@roughbits01 roughbits01 commented Feb 20, 2020

Add security signals and RUM events as data sources for Timeseries, Query Value, Table and Top List dashboard widgets.

PS: The terraform-provider-datadog PR

Copy link
Collaborator

@nmuesch nmuesch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. I believe these should also be added to -

ScatterPlotRequest
HeatMapWidgetRequest
DistributionWidgetRequest
HostMapRequest

Copy link
Collaborator

@nmuesch nmuesch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this pretty much looks good to go for me. Sorry for not mentioning it sooner, but could we add a test, maybe similar to https://github.com/zorkian/go-datadog-api/blob/master/integration/dashboards_test.go#L337 that just tries to create these new request types?

@@ -334,6 +334,68 @@ func createGraphWithLogQuery() []datadog.Graph {
return graphs
}

func createGraphWithRumQuery() []datadog.Graph {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these. These should be called inside a method Test... similar to

func TestDashboardCreateWithProcessQuery(t *testing.T) {

Copy link
Collaborator

@nmuesch nmuesch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for this!
I ran the acceptance (integration) tests locally and can confirm these new tests are passing.

@nmuesch nmuesch merged commit b397937 into zorkian:master Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants