fix(telemetry): Application Insights Shutdown on Retina Operator #1227
Conversation
mereta
changed the title
There was a problem hiding this comment.
Can you also make the relevant changes for the non-legacy (cilium-crd) operator? Up to you if it should be the same PR or separate.
In your testing, could you build a new retina image with your changes, deploy it in your cluster, and induce an error to verify that the final error log shows up in app insights? Could induce an error by modifying the code for testing.
operator/cmd/legacy/deployment.go
Outdated
| @@ -176,10 +165,14 @@ func (o *Operator) Start() { | |||
| "version": buildinfo.Version, | |||
| telemetry.PropertyApiserver: apiserverURL, | |||
| } | |||
|
|
|||
| telemetry.InitAppInsights(buildinfo.ApplicationInsightsID, buildinfo.Version) | |||
There was a problem hiding this comment.
I thought we verified in the past that operator was sending logs to app insights. I'm confused what this function does
We also seem to init app insights here and with a different config:
Lines 95 to 100 in a2c261f
@jimassa / @matmerr what's the difference between configuring zap with AI vs initializing AI client? What's the impact of having different configs?
Co-authored-by: Timothy J. Raymond <[email protected]> Signed-off-by: Mereta <[email protected]>
mereta
force-pushed
the
telemetry-shutdown
branch
from
f58fd99 to
454140e
Compare
Description
Fix for missing telemetry after application shutdown.
This pull request includes changes to improve error handling in the
operator/cmd/legacy/deployment.goandoperator/cmd/root.gofiles. The main focus is to replaceos.Exit(1)calls with error returns to allow for better error management and testing.Error handling improvements:
operator/cmd/legacy/deployment.go: Modified theStartmethod to return an error instead of callingos.Exit(1)on failure. This change affects multiple points within the method where errors are now returned instead of causing the program to exit.operator/cmd/root.go: Updated theRunfunction toRunEto handle errors returned by theStartmethod of theOperator. This ensures that any errors encountered during the startup process are properly returned and handled.Related Issue
#1203
Checklist
git commit -S -s ...). See this documentation on signing commits.Screenshots (if applicable) or Testing Completed
Please add any relevant screenshots or GIFs to showcase the changes made.
Additional Notes
Add any additional notes or context about the pull request here.
Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.