Skip to content
This repository was archived by the owner on Sep 17, 2025. It is now read-only.

Attach rate metrics for VM#935

Merged
lzchen merged 6 commits intocensus-instrumentation:masterfrom
lzchen:vm
Jul 23, 2020
Merged

Attach rate metrics for VM#935
lzchen merged 6 commits intocensus-instrumentation:masterfrom
lzchen:vm

Conversation

@lzchen
Copy link
Contributor

@lzchen lzchen commented Jul 22, 2020

Continuation of [#930].

Detects whether current app is hosted in an Azure VM. We utilize AzureMetaDataService to retrieve vm information. We need to periodically retrieve this (not only on load) because this information can change.

response = requests.get(request_url, headers={"MetaData": "True"})
except requests.exceptions.ConnectionError:
# Not in VM
self.is_vm = False

Choose a reason for hiding this comment

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

Is connection error enough to determine is not a VM?, maybe adding generic except as well to be sure doesn't think is a VM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct about the second except. I'll add it to catch anything else.

self.properties.clear()
self.update_properties()
self.heartbeat.clear()
# pylint: disable=protected-access

Choose a reason for hiding this comment

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

Is no other way to update the label keys value?, maybe creating the LongGauge again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking that maybe this would eventually take up too much memory, but Python gc is pretty efficient so we can do it this way.

Copy link

@hectorhdzg hectorhdzg left a comment

Choose a reason for hiding this comment

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

LGTM, just minor not blocking comments

@lzchen lzchen merged commit a4496dd into census-instrumentation:master Jul 23, 2020
@lzchen lzchen deleted the vm branch July 23, 2020 16:25
This was referenced Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants