-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add basic metric for Containers on Windows #1
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build !linux | ||
// +build !linux | ||
|
||
package host // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awscontainerinsightreceiver/internal/host" | ||
|
||
// These variables are invalid for Windows | ||
const ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another nit: This works, but could there be other more readable (and maintainable) ways to refactor this and enable the ability of having OS-specific behaviour or properties? Not sure how the codebase is what might work best so would rely on your good judgement. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw similar pattern of defining OS specific const in repo. |
||
rootfs = "" | ||
hostProc = rootfs + "" | ||
hostMounts = hostProc + "" | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build !windows | ||
// +build !windows | ||
|
||
package host // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awscontainerinsightreceiver/internal/host" | ||
|
||
const ( | ||
rootfs = "/rootfs" // the root directory "/" is mounted as "/rootfs" in container | ||
hostProc = rootfs + "/proc" // "/rootfs/proc" in container refers to the host proc directory "/proc" | ||
hostMounts = hostProc + "/mounts" // "/rootfs/proc/mounts" in container refers to "/proc/mounts" in the host | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build windows | ||
// +build windows | ||
|
||
package k8swindows // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awscontainerinsightreceiver/internal/k8swindows" | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Can we ignore building this on Linux?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, addressed! |
||
import ( | ||
"context" | ||
"errors" | ||
"os" | ||
|
||
ci "github.com/open-telemetry/opentelemetry-collector-contrib/internal/aws/containerinsight" | ||
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awscontainerinsightreceiver/internal/cadvisor/extractors" | ||
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awscontainerinsightreceiver/internal/host" | ||
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awscontainerinsightreceiver/internal/stores" | ||
|
||
"go.opentelemetry.io/collector/pdata/pmetric" | ||
"go.uber.org/zap" | ||
) | ||
|
||
type K8sWindows struct { | ||
cancel context.CancelFunc | ||
logger *zap.Logger | ||
nodeName string `toml:"node_name"` | ||
k8sDecorator stores.K8sDecorator | ||
summaryProvider *kubeletSummaryProvider | ||
hostInfo host.Info | ||
} | ||
|
||
func New(logger *zap.Logger, decorator *stores.K8sDecorator, hostInfo host.Info) (*K8sWindows, error) { | ||
nodeName := os.Getenv("HOST_NAME") | ||
if nodeName == "" { | ||
return nil, errors.New("missing environment variable HOST_NAME. Please check your deployment YAML config") | ||
} | ||
k8sSummaryProvider, err := new(logger, hostInfo) | ||
if err != nil { | ||
logger.Error("failed to initialize kubelet summary provider, ", zap.Error(err)) | ||
return nil, err | ||
} | ||
return &K8sWindows{ | ||
logger: logger, | ||
nodeName: nodeName, | ||
k8sDecorator: *decorator, | ||
summaryProvider: k8sSummaryProvider, | ||
hostInfo: hostInfo, | ||
}, nil | ||
} | ||
|
||
func (k *K8sWindows) GetMetrics() []pmetric.Metrics { | ||
k.logger.Debug("D! called K8sWindows GetMetrics") | ||
var result []pmetric.Metrics | ||
|
||
metrics, err := k.summaryProvider.getMetrics() | ||
if err != nil { | ||
k.logger.Error("error getting metrics from kubelet summary provider, ", zap.Error(err)) | ||
return result | ||
} | ||
metrics = k.decorateMetrics(metrics) | ||
for _, k8sSummaryMetric := range metrics { | ||
md := ci.ConvertToOTLPMetrics(k8sSummaryMetric.GetFields(), k8sSummaryMetric.GetTags(), k.logger) | ||
result = append(result, md) | ||
} | ||
|
||
return result | ||
} | ||
|
||
func (c *K8sWindows) decorateMetrics(cadvisormetrics []*extractors.CAdvisorMetric) []*extractors.CAdvisorMetric { | ||
//ebsVolumeIdsUsedAsPV := c.hostInfo.ExtractEbsIDsUsedByKubernetes() | ||
var result []*extractors.CAdvisorMetric | ||
for _, m := range cadvisormetrics { | ||
tags := m.GetTags() | ||
//c.addEbsVolumeInfo(tags, ebsVolumeIdsUsedAsPV) | ||
|
||
// add version | ||
//tags[ci.Version] = c.version | ||
|
||
// add nodeName for node, pod and container | ||
metricType := tags[ci.MetricType] | ||
if c.nodeName != "" && (ci.IsNode(metricType) || ci.IsInstance(metricType) || | ||
ci.IsPod(metricType) || ci.IsContainer(metricType)) { | ||
tags[ci.NodeNameKey] = c.nodeName | ||
} | ||
|
||
// add instance id and type | ||
if instanceID := c.hostInfo.GetInstanceID(); instanceID != "" { | ||
tags[ci.InstanceID] = instanceID | ||
} | ||
if instanceType := c.hostInfo.GetInstanceType(); instanceType != "" { | ||
tags[ci.InstanceType] = instanceType | ||
} | ||
|
||
// add scaling group name | ||
tags[ci.AutoScalingGroupNameKey] = c.hostInfo.GetAutoScalingGroupName() | ||
|
||
// add tags for EKS | ||
tags[ci.ClusterNameKey] = c.hostInfo.GetClusterName() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Seems like you covered the critical tags. Additional tags to consider if not already added. Might edit and add more as I think of more ideas
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. information about region is already part metrics internally, information about OS will be included in future PRs when refining metric labels |
||
|
||
out := c.k8sDecorator.Decorate(m) | ||
if out != nil { | ||
result = append(result, out) | ||
} | ||
} | ||
return result | ||
} | ||
|
||
func (k *K8sWindows) Shutdown() error { | ||
k.logger.Debug("D! called K8sWindows Shutdown") | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build !windows | ||
// +build !windows | ||
|
||
package k8swindows // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awscontainerinsightreceiver/internal/k8swindows" | ||
|
||
import ( | ||
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awscontainerinsightreceiver/internal/host" | ||
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/awscontainerinsightreceiver/internal/stores" | ||
|
||
"go.opentelemetry.io/collector/pdata/pmetric" | ||
"go.uber.org/zap" | ||
) | ||
|
||
type K8sWindows struct { | ||
} | ||
|
||
// New is a dummy function to construct a dummy K8sWindows struct for linux | ||
func New(_ *zap.Logger, _ *stores.K8sDecorator, _ host.Info) (*K8sWindows, error) { | ||
return &K8sWindows{}, nil | ||
} | ||
|
||
// GetMetrics is a dummy function to always returns empty metrics for linux | ||
func (k *K8sWindows) GetMetrics() []pmetric.Metrics { | ||
return []pmetric.Metrics{} | ||
} | ||
|
||
func (k *K8sWindows) Shutdown() error { | ||
return nil | ||
} |
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.
Intereseting! I wonder if we need to make the concept of a "cAdvisor metric" more generic, basically it is a container insights metric or container insights system metric. Then that could sit in
internal
and be shared by both the cAdvisor and windows impl.For now though I think it is fine to just do this, that is a decent refactor for not a ton of value other than better semantics