- 
                Notifications
    You must be signed in to change notification settings 
- Fork 151
[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
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
2721ea2    to
    4d10e0d      
    Compare
  
    | All the uses cases with their respective screenshots were updated in the comment above. Most of them went as expected. However, about  Additionally, if you see under the traces list, the naming is not really descriptive  On the other hand, by checking the attribute limits (the information populated under every single span), looks like it's configurable (Attribute Limits). If it exceeds the limit set, it truncates the content, therefore I think the event message could be populated, as well. At this moment, I removed it from the attributes, could be easily added at any point in time | 
| Setting otel service name to  We could also explore the option of sourcing the service name entirely from  | 
| Resuming the discussion we had some time ago, this implementation does not support parent-child relationship across spans. Currently, the code forces a specific value for traceID (hash based on the convention:  OTEL also allows to force spanID, however this one needs to be already present in the system (Jaeger or any other OTEL Collector), because that's the way to establish a parent-child relationship, refer to an existing spanID. Coming back to our use case, if we would like to have deeper level in our spans, we should think a way to establish that relationship. For instance and following the first use case above, have a  Not necessarily needs to be tackled here in this PR, just to kick off the discussion a bit 😃. Any thoughts on this? @stefanprodan @matheuscscp | 
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.
LGTM! 🚀
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 add otel to the provider docs and explain what it does
| 
 Sorry, I missed to add the documentation. Just added now 😄 . | 
Signed-off-by: Adrian Fernandez De La Torre <[email protected]>
| Hey @adri1197, I have one last question for this PR: The screenshots above are proving that after notification-controller has sent all the events of a particular  | 
| 
 Hi @matheuscscp! 😃 Yes, based on the use cases discussed. Perhaps, the top screenshots does not depict the whole information properly. I would suggest you if you can take a look at others shared below. All the spans you see on each screenshot belong to a unique trace (and therefore, they have a shared traceID - rootSpan). | 
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.
LGTM
Thanks @adri1197 🥇
Implementation of: fluxcd/flux2#5321
Closes: #1097
Use Cases
GitRepository->Kustomization->ConfigMapOCIRepository>HelmReleaseOCIRepository-
revision:6.9.1@sha256:565d310746f1fa4be7f93ba7965bb393153a2d57a15cfe5befc909b790a73f8aHelmRelease-
revision:6.9.1+565d310746f1-
oci-digest:sha256:565d310746f1fa4be7f93ba7965bb393153a2d57a15cfe5befc909b790a73f8a-
app-version:6.9.1HelmChart->HelmReleasesPart of: #1097
Part of: fluxcd/flux2#5321