Repository navigation
Conversation
|
Closes #1 |
| regex += "|" | ||
| } | ||
| regex += h | ||
| } |
There was a problem hiding this comment.
the whole foor loop could be regex := strings.Join(hostnames, "|")
| } | ||
| regex += h | ||
| } | ||
| return fmt.Sprintf("%v{vcluster=\"%v\", hostname=~\"%v\"}", metric, vcluster, regex) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
At least log that this value could not be casted to the expected format.
| } | ||
| valStr, ok := pair[1].(string) | ||
| if !ok { | ||
| continue |
| idx := int(ts.Sub(from).Seconds() / stepDuration.Seconds()) | ||
| if idx >= 0 && idx < numPoints { | ||
| ret.MetricByNode[hostname][gpuID].Data[idx] = val | ||
| } |
There was a problem hiding this comment.
else log that sth is out of bounds
|
|
||
| info, ok := victoriametrics.SupportedMetrics[metricName] | ||
| if !ok { | ||
| pie(logger.Warn, condition_error{fmt.Sprintf("Unsupported VM DCGM metric: %v", metricName)}, "", http.StatusBadRequest) |
| 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 |
There was a problem hiding this comment.
this comment applies to other handlers too. It's always the same duplicate check
| } | ||
|
|
||
| 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) |
There was a problem hiding this comment.
No VM for user error messages.
Currently, only 4 metrics are added:
gpu_temp,memory_temp,power_usageandgpu_utilization. Adding other VM metrics is easily done invictoriametrics.go.