weaveworks / grafanalib

Python library for building Grafana dashboards
Apache License 2.0
1.86k stars 309 forks source link

ElasticsearchTarget helper class fails isinstance(Target) validation in AlertRules #553

Open lockan opened 1 year ago

lockan commented 1 year ago

What you expected to happen?

Grafana 9.2.4 Grafanalib 0.7.0

When trying to define an AlertRulev9 object for use with an Elasticsearch data source the Elasticsearch helper classes should pass validation tests and correctly generate the JSON structure needed to provision a new working alert.

What happened?

Trying to the ElasticsearchTarget in the triggers block of the AlertRulev9 results in the error:

ValueError: triggers must either be a Target or AlertCondition

This appears to be because the validation check is looking for isinstance(Target) or ..., but the class ElasticsearchTarget inherits from Object, not Target.

I have also tried using the core Target class and passing the ElasticsearchTarget in to the target param. e.g.

Target(
   ...
   target=ElasticsearchTarget( ... ),

)

If I do this my code will execute and I can provision the alert. However the resulting alert fails to parse correctly in Grafana with an error:

Failed to execute conditions: [plugin.downstreamError] failed to query data: type assertion to string failed.

How to reproduce it?

AlertRulev9(
    title="My provisioned Alert Rule",
    #condition="B",
    evaluateFor="5m",
    timeRangeFrom=300,
    timeRangeTo=0,
    noDataAlertState=ALERTRULE_STATE_DATA_OK,
    errorAlertState=ALERTRULE_STATE_DATA_ERROR,
    dashboard_uid="${REALLYREALDASBOARDUID}",
    triggers=[
        ElasticsearchTarget(
            refId="A",
            alias="",
            datasource="${VALID_OPENSEARCH_SRC_UID}",
            query = "userIdentity.type:Root"
            bucketAggs=[
                DateHistogramGroupBy(
                    field="@timestamp",
                    minDocCount=0,
                )
            ],
            metricAggs=[CountMetricAgg()],
        timeField="@timestamp",
        ).auto_bucket_agg_ids(),
        ElasticsearchAlertCondition(
            target=estarget,
            evaluator=GreaterThan(0),
            operator=OP_AND,
            reducerType=RTYPE_SUM,
        )

    ],
    annotations={
        "foor": "bar",
    },
    labels={
        "environment": "dev",
        "slack": "alerts",
    }
)
mkotsalainen commented 1 year ago

The same problem affects CloudwatchMetricsTarget.

JamesGibo commented 1 year ago

CloudwatchMetricsTarget fixed in https://github.com/weaveworks/grafanalib/pull/560

Similar fix could be used for Elasticsearch

xFallingDuskx commented 1 year ago

Hi! I noticed that the same issue is still happening. Has there been any progress on a fix? Also, I was wondering where do we specify the datasource UID when using ElasticsearchTarget?

Edit: I see that just changing the type of ElasticsearchTarget to be Target completely solves the issue and allows for the specification of a datasource. Maybe this change, along with the fix for CloudwatchMetricsTarget, can be pushed to the grafanalib package.