-
Notifications
You must be signed in to change notification settings - Fork 29
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
Collect metrics asynchronously #223
Collect metrics asynchronously #223
Conversation
…nsure we have a fresh collection every
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.
LGTM
…that are missing at runtime
0cc11a1
to
5c3bdd5
Compare
…not supported' errors
client.logger.Warnf("Unable to query '%s' on '%s'", dcgmName, reason) | ||
if failedCount == maxWarningsForFailedDeviceMetricQuery { | ||
if failedCount <= limit { | ||
client.logger.Warnf("%s", reason) |
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.
logger.Warnf("%s", reason)
is the same as logger.Warn(reason)
(https://pkg.go.dev/go.uber.org/zap#SugaredLogger.Warn). But I can fix that after we merge this PR…
@@ -328,9 +328,11 @@ func (client *dcgmClient) collect() (time.Duration, error) { | |||
if err := isValidValue(fieldValue); err == errBlankValue { | |||
// Blank values are expected at startup. | |||
continue | |||
} else if err == errNotSupported { | |||
client.issueWarningForFailedQueryUptoThreshold(dcgmName, 1, fmt.Sprintf("Field '%s' is not supported. Metric '%s' will not be collected", dcgmName, dcgmName)) |
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.
The Metric '%s' will not be collected
part is redundant (in the profiling metrics as well). But I can fix that after we merge this PR…
No description provided.