-
Notifications
You must be signed in to change notification settings - Fork 149
[RFC-0011] - OTEL integration based on alerts #1149
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
base: main
Are you sure you want to change the base?
Conversation
6e2dbbd
to
7a793ec
Compare
261f89d
to
95a19f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass
Signed-off-by: Adrian Fernandez De La Torre <adri1197@gmail.com>
@@ -215,6 +216,7 @@ func (s *EventServer) dispatchNotification(ctx context.Context, event *eventv1.E | |||
|
|||
go func(n notifier.Interface, e eventv1.Event) { | |||
pctx, cancel := context.WithTimeout(context.Background(), timeout) | |||
pctx = context.WithValue(pctx, notifier.AlertMetadataContextKey{}, alert.ObjectMeta) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please swap this line with defer cancel()
apiv1beta3 "github.com/fluxcd/notification-controller/api/v1beta3" | ||
) | ||
|
||
type AlertMetadataContextKey struct{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type AlertMetadataContextKey struct{} | |
type alertMetadataContextKey struct{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, please revert the changes in this file, as otel
should be its own provider type
Summary
GitRepository
>Kustomization
>HelmRepository
>HelmRelease
Open Questions
v1/traces
(automatically managed by Otel GO SDK). In this way, it could allow that the notifications are sent to another endpoint, while the traces could be properly sent, as well (even though this endpoint convention must be met). Another alternative, could be to make it configurable by parsing the URL or some kind of convention?OtelTracer
, even though it has not been populated as a new provider type. The rationale is to be able to rolling it out to the current providers that may support OpenTelemetry. At this moment, justgeneric
type.Part of: #1097
Part of: fluxcd/flux2#5321