[processor/resourcedetection] introduce kubeadm detector #35450
Conversation
github-actions
bot
added
internal/metadataproviders
processor/resourcedetection
odubajDT
force-pushed
the
resourcedetection-local-cluster
branch
2 times, most recently
from
1bdc2fb
to
3c5422f
Compare
processor/resourcedetectionprocessor/internal/kubeadm/config.go
Outdated
Show resolved
Hide resolved
processor/resourcedetectionprocessor/internal/kubeadm/kubeadm.go
Outdated
Show resolved
Hide resolved
| if d.ra.K8sClusterName.Enabled { | ||
| clusterName, err := d.provider.ClusterName(ctx) | ||
| if err != nil { | ||
| return pcommon.NewResource(), "", fmt.Errorf("failed getting k8s cluster name: %w", err) |
There was a problem hiding this comment.
Nit, feel free to drop: I'm wondering if it would be better to future-proof this by not returning the error right away, and returning the error at the end.
If we end up adding more resource attributes the existing logic would not record the other, unrelated resource attributes, even if they could be detected and emitted successfully.
odubajDT
force-pushed
the
resourcedetection-local-cluster
branch
from
5dbc1c7
to
99bf118
Compare
processor/resourcedetectionprocessor/internal/kubeadm/config.go
Outdated
Show resolved
Hide resolved
odubajDT
force-pushed
the
resourcedetection-local-cluster
branch
2 times, most recently
from
062d966
to
80bd836
Compare
| defaultConfigMapName = "kubeadm-config" | ||
| defaultConfigMapNamespace = "kube-system" |
There was a problem hiding this comment.
Nit: Do these defaults need to be in the resource detection processor? Could they instead be moved to internal/metadataproviders/kubeadm/metadata.go? It doesn't appear to be necessary to pass these down to the NewProvider method, but maybe there's a scenario I'm not thinking of.
There was a problem hiding this comment.
The only scenario would be that we could make the configMap name and namespace configurable in the future. Currently that is not needed at all.
Technically we can move the default values to the internal package you mentioned, but I do not see much advantage technically there. Leaving it up to your decision.
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
odubajDT
force-pushed
the
resourcedetection-local-cluster
branch
from
6436f3a
to
0eda211
Compare
|
@crobert-1 any other suggestions? |
|
cc @crobert-1 |
Description:
kubeadmdetector to detect local cluster nameLink to tracking Issue: #35116