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

[IMPROVE] Detector integration_azure-key-vault -> remove critical for API Latency #464

Closed
freezeeedos opened this issue Apr 4, 2023 · 0 comments · Fixed by #465
Closed
Labels
detectors About nex or existing detectors enhancement Enhancement of existing (templating or detectors)

Comments

@freezeeedos
Copy link
Contributor

What is the module?
integration_azure-key-vault

What is the detector?
api_latency

Is your improvement related to a known monitoring limitations? Please describe.
The API_LATENCY detector should not raise alerts of type "critical". These alerts are rarely acted upon, and a "major" severity is sufficent

Describe the solution you'd like
Shifting the severities for this detector from MAJOR and CRITICAL to MINOR and MAJOR

In terraform-signalfx-detectors/modules/integration_azure-key-vault/variables.tf:
api_latency_threshold_critical -> api_latency_threshold_major
api_latency_threshold_major -> api_latency_threshold_minor
Remove api_latency_disabled_critical

In terraform-signalfx-detectors/modules/integration_azure-key-vault/detectors-keyvault.tf:

    severity              = "Critical"
    detect_label          = "CRIT"
    disabled              = coalesce(var.api_latency_disabled_critical, var.api_latency_disabled, var.detectors_disabled)
    notifications         = try(coalescelist(lookup(var.api_latency_notifications, "critical", []), var.notifications.critical), null)

To

    description           = "is too high > ${var.api_latency_threshold_major}ms"
    severity              = "Major"
    detect_label          = "MAJOR"
    disabled              = coalesce(var.api_latency_disabled_major, var.api_latency_disabled, var.detectors_disabled)
    notifications         = try(coalescelist(lookup(var.api_latency_notifications, "major", []), var.notifications.major), null)

And

    description           = "is too high > ${var.api_latency_threshold_major}ms"
    severity              = "Major"
    detect_label          = "MAJOR"
    disabled              = coalesce(var.api_latency_disabled_major, var.api_latency_disabled, var.detectors_disabled)
    notifications         = try(coalescelist(lookup(var.api_latency_notifications, "major", []), var.notifications.major), null)

Would become

    description           = "is too high > ${var.api_latency_threshold_major}ms"
    severity              = "Minor"
    detect_label          = "MINOR"
    disabled              = coalesce(var.api_latency_disabled_major, var.api_latency_disabled, var.detectors_disabled)
    notifications         = try(coalescelist(lookup(var.api_latency_notifications, "major", []), var.notifications.major), null)

Describe alternatives you've considered
Overriding the parameters on each individual stack, which is not practical

@freezeeedos freezeeedos added detectors About nex or existing detectors enhancement Enhancement of existing (templating or detectors) labels Apr 4, 2023
@freezeeedos freezeeedos linked a pull request Apr 5, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
detectors About nex or existing detectors enhancement Enhancement of existing (templating or detectors)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant