vSphere new implementation #5251
Conversation
FlorianVeaux
force-pushed
the
florian/vsphere_v2
branch
2 times, most recently
from
dc51c68 to
2e1e520
Compare
Codecov Report
|
FlorianVeaux
force-pushed
the
florian/vsphere_v2
branch
2 times, most recently
from
c2460dc to
bc27a62
Compare
Codecov Report
|
FlorianVeaux
force-pushed
the
florian/vsphere_v2
branch
from
c187963 to
ff82e48
Compare
| if not self.ssl_verify: | ||
| context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) | ||
| context.verify_mode = ssl.CERT_NONE | ||
| elif self.ssl_capath: | ||
| context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) | ||
| context.verify_mode = ssl.CERT_REQUIRED | ||
| context.load_verify_locations(capath=self.ssl_capath) | ||
|
|
There was a problem hiding this comment.
Try to copy what you can from
integrations-core/vertica/datadog_checks/vertica/vertica.py
Lines 569 to 590 in 018422d
There was a problem hiding this comment.
If that's something we do over and over. Can we extract that as a reusable util? (Probably in another PR)
Clean up config options Add dependency Re-implement excluded_host_tags work Fix tests Fetch remaining metrics Clean-up Fix dependencies fix style [review] max_query_metrics exception [review] Document self._conn.content type [review] Address minor comments
FlorianVeaux
force-pushed
the
florian/vsphere_v2
branch
from
388d977 to
bcd4714
Compare
|
Question: Should ressource_filters expect metrics to be prefixed with EDIT: Added documentation and leaving it as is |
|
Question2: Should we allow |
| mor_type = type(mor) | ||
| if mor_type not in self._content: | ||
| self._content[mor_type] = {} | ||
| self._content[mor_type][mor] = mor_data |
There was a problem hiding this comment.
Can we use something else than a MOR object as cache key for self._content[mor_type][mor] ?
Maybe use a "MOR Id".
There was a problem hiding this comment.
I looked into pyvmomi code https://github.com/vmware/pyvmomi/blob/master/pyVmomi/VmomiSupport.py#L596-L610.
What this means is that is it safe to use the mor directly as the cache key, the string representation of it should be unique so there should be no hash collision, and even in case of the same string for two different mors, the __eq__ method should prevent collisions.
So another possibility would be to use str(mor) as the cache key, but note that we would lose a bit in case two mors have the same string representation. Note also that the str method can be quite complex in regard of the very simple __hash__ and __eq__ methods.
Re-implementing the vSphere integration for multiple reasons:
Missing parts that will come in separate PRs:
vsphere.cpu.usagecan be tagged withinstance:%dwhere the integer is the CPU core identifier). For now we do not collect those as it is expensive but we should add the option to collect them. Will be in a separate PR.Major things to look for:
Note: Because we moved the legacy
vsphere.pyfile tovsphere_legacy.pyand because the new implementation also has to be namedvsphere.py, git tries to show a diff between the two implementation which is a bit confusing. You can look the latest version of thevsphere.pyfile just hereNote2: The integration is running against our lab in our
dddevenvironment.Note3: I haven't checked yet if metadata.csv is up to date but in theory it should.
Note4: Don't mind the number of lines, most of it is either big fixture files or just moving the legacy code somewhere else.