-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(azure): add vm_id to KVP telemetry event keys #6551
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
| "Query for VM ID returned empty. Using zero-guid." | ||
| ) | ||
| vm_id = self.ZERO_GUID | ||
|
|
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.
can we also log the event where query for vm id was successful?
cloudinit/reporting/handlers.py
Outdated
| ) | ||
|
|
||
| try: | ||
| from cloudinit.sources.azure.identity import query_vm_id |
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.
was this done late to avoid circular imports?
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.
Yeah that was exactly why - I kept getting an error message about that. Let me know if there is a better way to avoid that circular import!
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.
Generally a circular import error indicates a lack of clear hierarchy in the code. The reporting code is currently not coupled to any particular cloud, but after this change that would no longer be true.
I haven't put much thought into it, but since query_vm_id is currently not used anywhere it could just get relocated to this module.
…rder Relocate query_vm_id() and helper functions from azure.identity to reporting.handlers to eliminate circular import. Also fix event key format to place vm_id before uuid in order to prevent breaking RdAgent.
5e5a253 to
7b66c56
Compare
|
A request was made outside of this PR to re-order |
Proposed Commit Message
Additional Context
The VM ID is now queried during
HyperVKvpReportingHandlerinitialization and appended to each event key.Changes include:
vm_idduringHyperVKvpReportingHandlerinitializationvm_idas instance attribute with fallback tozero-guidstring_event_key_format_parts()to includevm_idin key structureCLOUD_INIT|<incarnation>|<event_type>|<event_name>|<uuid>|<vm_id>[|subevent_index]test_vm_id_fallback_to_zero_guidunit test to verify fallback logic.vm_idin key, wherevm_idis 26bd6c2f-ecc6-4b04-9f79-e500096ee45b:Merge Type