Skip to content

Add VM endpoint and DCGM metrics - #3

Open
sgozel wants to merge 4 commits into
finkandreas:mainfrom
sgozel:1-add-vm-data-source
Open

sgozel wants to merge 4 commits into
finkandreas:mainfrom
sgozel:1-add-vm-data-source

Conversation

@sgozel

@sgozel sgozel commented Oct 12, 2026

Copy link
Copy Markdown

Currently, only 4 metrics are added: gpu_temp, memory_temp, power_usage and gpu_utilization. Adding other VM metrics is easily done in victoriametrics.go.

@sgozel

sgozel commented Oct 12, 2026

Copy link
Copy Markdown
Author

Closes #1

@sgozel sgozel mentioned this pull request Oct 16, 2026
regex += "|"
}
regex += h
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

the whole foor loop could be regex := strings.Join(hostnames, "|")

}
regex += h
}
return fmt.Sprintf("%v{vcluster=\"%v\", hostname=~\"%v\"}", metric, vcluster, regex)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

If you use quotes in a string it is often easier to use raw strings, i.e. backticks. This would be equivalent:

	return fmt.Sprintf(`%v{vcluster="%v", hostname=~"%v"}`, metric, vcluster, regex)


info, ok := SupportedMetrics[metricName]
if !ok {
return nil, fmt.Errorf("Unsupported VM DCGM metric: %v", metricName)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I would avoid in the error message the word VM, just Unsupported DCGM metric: %v.
A user potentially would see an error message, and they surely have no idea what VM would mean in the API request context.

}
tsFloat, ok := pair[0].(float64)
if !ok {
continue

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

At least log that this value could not be casted to the expected format.

}
valStr, ok := pair[1].(string)
if !ok {
continue

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

same as above

idx := int(ts.Sub(from).Seconds() / stepDuration.Seconds())
if idx >= 0 && idx < numPoints {
ret.MetricByNode[hostname][gpuID].Data[idx] = val
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

else log that sth is out of bounds

Comment thread handler/vm_dcgm.go

info, ok := victoriametrics.SupportedMetrics[metricName]
if !ok {
pie(logger.Warn, condition_error{fmt.Sprintf("Unsupported VM DCGM metric: %v", metricName)}, "", http.StatusBadRequest)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Skip the word VM

Comment thread handler/vm_dcgm.go
cluster_config, err := h.config.GetClusterConfig(systemName)
pie(logger.Warn, err, "", http.StatusBadRequest)

// [TO DO] 8 lines below are taken from dcgm.go ---> refactor to avoid duplicate

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this comment applies to other handlers too. It's always the same duplicate check

Comment thread handler/vm_dcgm.go
}

dcgmData, err := h.vmclient.GetDcgmMetric(metricName, nodes, from, to, cluster_config.Name, logger)
pie(logger.Error, err, fmt.Sprintf("Failed getting VM DCGM metric %v", metricName), http.StatusInternalServerError)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

No VM for user error messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants