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

DEV-46341: add logzio opsgenie support #40

Conversation

Jonathan-Eng
Copy link
Collaborator

@Jonathan-Eng Jonathan-Eng commented Sep 10, 2024

add support for logzio opsgenie integration (v1 variant)

@Jonathan-Eng Jonathan-Eng changed the title add opsgenie v1 support - url and authentication DEV-46341: add logzio opsgenie support Sep 16, 2024
@Jonathan-Eng
Copy link
Collaborator Author

run-test

1 similar comment
@Jonathan-Eng
Copy link
Collaborator Author

run-test

@Jonathan-Eng Jonathan-Eng force-pushed the DEV-46341-create-logzio-opsgenine-notification-endpoint-option branch from 5e4261a to 0cf061a Compare September 26, 2024 08:42
Copy link

@yasmin-tr yasmin-tr left a comment

Choose a reason for hiding this comment

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

the same comment as other PR about adding the logz grafana change comment with reference to ticket

also, i'm not sure why but i don't see the grafana jenkins pipeline ran .
I want us to make sure no new tests are failing, and fix them if there are.
(to see the previous failing you can take a look at the last run in the main branch. i hope it is still available)

@@ -147,7 +146,7 @@ export function ChannelSubForm<R extends ChannelValues>({
inputId={contactPointTypeInputId}
{...field}
width={37}
options={typeOptions.filter((o) => !o.value.startsWith(INTERNAL_CHANNEL_TYPE_PREFIX))} // LOGZ.IO GRAFANA CHANGE :: DEV-35483 - Filter out logzio opsgenie type from creation
options={typeOptions}

Choose a reason for hiding this comment

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

why remove this ? 🤔 don't we want same behaviour as before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, we want to support v1. decision was made by @ohadza and Amos

@@ -519,6 +519,6 @@ replace xorm.io/xorm => ./pkg/util/xorm
// This is required in order to get notification delivery errors from the receivers API.
replace github.com/prometheus/alertmanager => github.com/grafana/prometheus-alertmanager v0.25.1-0.20240208102907-e82436ce63e6

replace github.com/grafana/alerting => github.com/logzio/data-viz-alerting v0.0.0-20240709132848-5bf841d2c3d3

Choose a reason for hiding this comment

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

just to make sure - i guess once you are merging the change in grafana-alerting , you'll also need to update the commit here (and on the go.sum) ?
otherwise you'd be referring to the branch and not "master" version 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, there will be another commit. you are right

@Jonathan-Eng
Copy link
Collaborator Author

run-test

@@ -1339,6 +1339,35 @@ func GetAvailableNotifiers() []*NotifierPlugin {
},
},
},
{
Type: "logzio_opsgenie",
Name: "LogzioOpsGenie",
Copy link
Collaborator Author

@Jonathan-Eng Jonathan-Eng Sep 26, 2024

Choose a reason for hiding this comment

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

Is this name agreeable? Hope it doesn't require refactor if not. @yasmin-tr @ohadza

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is customer facing

Choose a reason for hiding this comment

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

btw - the "logzio_opsgenie" type today as well appears on the api , so customers will see that anyway.
if it is correct to call it then maybe it's ok. i guess we just need to make sure it looks ok on the UI as "LogzioOpsGenie" (maybe it needs to be with space? or some icon even? 🤷‍♀️ )

i'd verify with Amos (product)

@Jonathan-Eng
Copy link
Collaborator Author

run-test

@Jonathan-Eng Jonathan-Eng merged commit 523d10f into v10.4.x-logzio Sep 26, 2024
10 of 15 checks passed
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