-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Prevent hostname evaluating to None in sqlserver check #18237
Conversation
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.
After merging, can we please add a test?
merged this one instead #18249 so i can deal with the tests separately. I cannot easily run the tests / fix them tonight and this is blocking the agent release |
2214285
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
Can you link to the Jira card? |
What does this PR do?
This change prevents a very bad bug where the host field on metrics and events will fall back to
None
when the static information cache reaches its TTL of 1 day. When this happens, we see gaps in data for sqlserver instances because our backend drops these as bad payloads. We can see an example of this happening in our integration environment:... meanwhile, if we log the payloads we see the
host
value is set tonull
:Motivation
https://datadoghq.atlassian.net/browse/DBMON-4711
In this previous change #17750 we updated the check to call
on each check run. This function does a few things:
self._resolved_hostname
value toNone
, but it never initializes the valueThis makes the
load_static_information
not safe to call outside of the context of theset_resolved_hostname
function. In order to prevent the resolved_hostname property from ever returningNone
we add a null check && re-initialize the value if necessary:Additional Notes
Review checklist (to be filled by reviewers)
qa/skip-qa
label if the PR doesn't need to be tested during QA.backport/<branch-name>
label to the PR and it will automatically open a backport PR once this one is merged