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

Doesn't support Log Alert Monitor( Fix log monitor UI issue, lost query tags) #148

Closed
geekeren opened this issue Jan 30, 2019 · 10 comments
Closed

Comments

@geekeren
Copy link
Contributor

geekeren commented Jan 30, 2019

Hi there, I create two monitors, one is created manually, the other is by terraform.

Terraform Version

Terraform v0.11.11

  • provider.datadog v1.6.0

Terraform Configuration Files

resource "datadog_monitor" "return_404s" {
  name               = "LB returning 404s"
  type               = "log alert"
  message            = <<EOT
The threshold on this may need to be tweaked!

@pagerduty-EdgeLoadBalancer
EOT
  query = "logs(\"source:nginx AND @http.status_code:404\").index(\"main\").rollup(\"count\").last(\"5m\") > 5"

  thresholds {
    critical = 5
  }
  silenced {
    "*" =  false ## unmute or not
  }
}

When I edit the monitor, I expected to see:

Expected Behavior

Expected Behavior

Actual Behavior

but I saw the search query is empty:

image

When I go to view page, the two monitors are the same:
image

I am not sure whether is UI issues of datadog, or the log alert doesn't work at all

@geekeren geekeren changed the title Log alert monitor doesn't work Doesn't support Log Alert Monitor Jan 31, 2019
@rsun-thoughtworks
Copy link

I have the same problem.
To find the root cause of it, I read the source code of terraform-provider and the POST message when we save a monitor in datadog page.

I found out that the "AND"s are in queryConfig.queryString and terraform-provider does not support this property.

I suggest that datadog official team can fix this bug.

Thanks for your team and work with love.

@h3x89
Copy link

h3x89 commented May 8, 2019

Do you have any plans to fix it? in provider.datadog v1.8.0 I still have this issue

@bkabrda
Copy link
Contributor

bkabrda commented May 9, 2019

Hi everyone, I’ve raised this issue internally to our monitors team and they’ve acknowledged this is a bug and will work to fix it. I'll post an update here when there's some development on this.

@geekeren
Copy link
Contributor Author

geekeren commented May 9, 2019

Hi everyone, I’ve raised this issue internally to our monitors team and they’ve acknowledged this is a bug and will work to fix it. I'll post an update here when there's some development on this.

@bkabrda Can I ask your team will fix this at the datadog-side or terraform datadog provider side? Because I am trying to fix this by changing code of terraform-datadog-provider then create a PR?

@bkabrda
Copy link
Contributor

bkabrda commented May 9, 2019

@geekeren they're still looking into it and trying to figure out what would be the best solution. ATM it's not been decided where the fix will be, so it can be either the provider, the datadog internal mechanisms or both. I'll keep updating this issue as situation develops. Thanks for your patience everyone!

geekeren pushed a commit to geekeren/terraform-provider-datadog that referenced this issue May 9, 2019
@geekeren
Copy link
Contributor Author

geekeren commented May 9, 2019

#199 Here is a PR to fix this issue

geekeren pushed a commit to geekeren/terraform-provider-datadog that referenced this issue May 9, 2019
@bkabrda
Copy link
Contributor

bkabrda commented May 14, 2019

Thanks for the PR, I'll try to have a look during today.

@bkabrda
Copy link
Contributor

bkabrda commented May 15, 2019

So after some discussion with the Datadog monitors team, it seems that the solution provided in #199 is correct short-term. In the long term, the monitors team will remove the need to add query_config and the log query will be parsed directly from the current query attribute (IOW, whenever this is fixed, we'll just revert #199 and things will continue working).

In terms of backwards compatibility, the monitors API will keep accepting (and silently ignoring) query_config for some time (which we'll make sure is long enough) after the fix explained above is implemented. That will make it safe for us to revert #199 and release new version that doesn't use it any more.

bkabrda pushed a commit that referenced this issue May 15, 2019
…query (#199)

* #148 Fix log monitor UI issue: generate queryConfig.queryString from query

* #148 make schema query_config computed

* refator match log queryString
@bkabrda
Copy link
Contributor

bkabrda commented May 15, 2019

The fixing PR was merged, I'm closing this issue.

@bkabrda bkabrda closed this as completed May 15, 2019
@geekeren geekeren changed the title Doesn't support Log Alert Monitor Doesn't support Log Alert Monitor( Fix log monitor UI issue, lost query tags) May 16, 2019
@bkabrda
Copy link
Contributor

bkabrda commented Jun 5, 2019

Hey folks, for the record, the API now parses the query itself and so query_config is not required. As a result:

  • I submitted PR#226 to revert PR#199 as it's no longer needed.
  • The previously released datadog provider versions should work fine from now on.

bkabrda pushed a commit that referenced this issue Jun 5, 2019
Revert "#148 Fix log monitor UI issue: generate queryConfig.queryStri…
jbenais pushed a commit to jbenais/terraform-provider-datadog that referenced this issue Aug 20, 2019
…g from query (DataDog#199)

* DataDog#148 Fix log monitor UI issue: generate queryConfig.queryString from query

* DataDog#148 make schema query_config computed

* refator match log queryString
jbenais pushed a commit to jbenais/terraform-provider-datadog that referenced this issue Aug 20, 2019
…eryString from query (DataDog#199)"

This reverts commit 7aade75.

The query_config parameter is no longer necessary, as all the attributes
are parsed out of the query parameter.
jbenais pushed a commit to jbenais/terraform-provider-datadog that referenced this issue Aug 20, 2019
Revert "DataDog#148 Fix log monitor UI issue: generate queryConfig.queryStri…
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

No branches or pull requests

4 participants