Skip to content

feat: Add adding alerting to k8s - #270

Merged
findingrish merged 2 commits into
mainfrom
alerting-helm
Jul 28, 2021
Merged

findingrish merged 2 commits into
mainfrom
alerting-helm

Conversation

@findingrish

@findingrish findingrish commented Jul 28, 2021

Copy link
Copy Markdown

Add alerting service to k8s

@JBAhire JBAhire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, once these small comments are addressed. :)

- name: hypertrace-alert-engine
repository: "https://storage.googleapis.com/hypertrace-helm-charts"
version: 0.1.4
condition: alerting.enabled

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we disable it by default?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The reason being, there won't be any slack URL and all so this will be failing anyways.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

it is disabled by default

@JBAhire JBAhire Jul 28, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cool. missed that last line.

},
"valueOperator": "VALUE_OPERATOR_EQ",
"rhsExpression": {
"stringValue": "customer"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems specifically tied to hotrod. Can we make it more abstract?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this is just some starter stuff, can be generalised later

@findingrish
findingrish merged commit 45eec6a into main Jul 28, 2021
@findingrish
findingrish deleted the alerting-helm branch July 28, 2021 12:52
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