-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[CONTP-277] add kubernetes_resources_annotations_as_tags and kubernetes_resources_labels_as_tags #27369
[CONTP-277] add kubernetes_resources_annotations_as_tags and kubernetes_resources_labels_as_tags #27369
Conversation
Regression DetectorRegression Detector ResultsRun ID: 537fd7f4-173f-4a42-a371-4b2b5adf874c Metrics dashboard Target profiles Baseline: e700016 Performance changes are noted in the perf column of each table:
No significant changes in experiment optimization goalsConfidence level: 90.00% There were no significant changes in experiment optimization goals at this confidence level and effect size tolerance.
|
perf | experiment | goal | Δ mean % | Δ mean % CI | links |
---|---|---|---|---|---|
➖ | tcp_syslog_to_blackhole | ingress throughput | +0.98 | [-11.81, +13.78] | Logs |
➖ | file_tree | memory utilization | +0.58 | [+0.51, +0.64] | Logs |
➖ | basic_py_check | % cpu utilization | +0.32 | [-2.28, +2.92] | Logs |
➖ | otel_to_otel_logs | ingress throughput | +0.06 | [-0.75, +0.88] | Logs |
➖ | idle | memory utilization | +0.03 | [-0.00, +0.06] | Logs |
➖ | uds_dogstatsd_to_api | ingress throughput | +0.00 | [-0.00, +0.00] | Logs |
➖ | tcp_dd_logs_filter_exclude | ingress throughput | -0.00 | [-0.01, +0.01] | Logs |
➖ | uds_dogstatsd_to_api_cpu | % cpu utilization | -0.89 | [-1.77, -0.00] | Logs |
➖ | pycheck_1000_100byte_tags | % cpu utilization | -1.67 | [-6.47, +3.12] | Logs |
Explanation
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
4bdcefc
to
0a1d42e
Compare
3dd5091
to
932a87b
Compare
5f1969e
to
0f4cd6a
Compare
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.
Suggesting some minor edits and clarification to the release note
releasenotes-dca/notes/add_generic_k8s_resource_tagging-64b51a2ffaf3e2cc.yaml
Outdated
Show resolved
Hide resolved
releasenotes-dca/notes/add_generic_k8s_resource_tagging-64b51a2ffaf3e2cc.yaml
Outdated
Show resolved
Hide resolved
releasenotes-dca/notes/add_generic_k8s_resource_tagging-64b51a2ffaf3e2cc.yaml
Outdated
Show resolved
Hide resolved
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.
@adel121 thank you for confirming that. I've left a final suggestion for additional clarification, but overall LGTM and approving.
- | | ||
Added capability to tag any Kubernetes resource based on labels and annotations. | ||
This feature can be configured with `kubernetes_resources_annotations_as_tags` and `kubernetes_resources_labels_as_tags`. | ||
These feature configurations are maps from group resource to a map of annotations/labels to tag. |
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.
These feature configurations are maps from group resource to a map of annotations/labels to tag. | |
These feature configurations associate a group resource with a map of annotations or labels to convert to tags. |
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.
with a map of annotations or labels to convert to tags.
This is not very accurate.
I made a modification to it, let me know if it makes sense to you.
Added capability to tag any Kubernetes resource based on labels and annotations. | ||
This feature can be configured with `kubernetes_resources_annotations_as_tags` and `kubernetes_resources_labels_as_tags`. | ||
These feature configurations are maps from group resource to a map of annotations/labels to tag. | ||
For example, `deployments.apps` can be mapped to an annotations-to-tags map to configure annotations as tags for deployments. |
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.
For example, `deployments.apps` can be mapped to an annotations-to-tags map to configure annotations as tags for deployments. | |
For example, `deployments.apps` can be associated with an annotations-to-tags map to configure annotations as tags for deployments. |
Test changes on VMUse this command from test-infra-definitions to manually test this PR changes on a VM: inv create-vm --pipeline-id=42136011 --os-family=ubuntu Note: This applies to commit 9042bdd |
cd210f8
to
3740333
Compare
3740333
to
133fbf1
Compare
3ab05c3
to
133fbf1
Compare
2dfa4e2
to
133fbf1
Compare
133fbf1
to
dec764e
Compare
The given example is injecting kubernetes_resources_labels_as_tags from env. Do we have plan to support config yaml? |
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
9614d91
to
2ea8eec
Compare
2ea8eec
to
9042bdd
Compare
Serverless Benchmark Results
tl;drUse these benchmarks as an insight tool during development.
What is this benchmarking?The The benchmark is run using a large variety of lambda request payloads. In the charts below, there is one row for each event payload type. How do I interpret these charts?The charts below comes from The benchstat docs explain how to interpret these charts.
I need more helpFirst off, do not worry if the benchmarks are failing. They are not tests. The intention is for them to be a tool for you to use during development. If you would like a hand interpreting the results come chat with us in Benchmark stats
|
/merge |
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
…es_resources_labels_as_tags (#27369)
What does this PR do?
Adds the capability to configure
kubernetes_resources_labels_as_tags
andkubernetes_resources_annotations_as_tags
.We already have the following possible configuration options:
The 2 added configuration options should become the preferred way to configure kubernetes resource tagging based on labels and/or annotations.
Motivation
Be able to generically choose how to tag kubernetes resources based on their annotations and/or labels.
Additional Notes
In order to preserve backward compatibility, we still support the old configuration options.
In the event where the user configures both the old and new configurations, the agent will merge all configurations into one config while giving precedence to the new config option in case of conflict by key. (See QA section for examples).
Possible Drawbacks / Trade-offs
To simplify, we modified workloadmeta collector factories so that the metadata collector never collects pods or deployments.
Pods and Deployments will always have their own dedicated collectors.
This simplification helps ensure we have a single source of truth for pod and deployment data in workloadmeta, which translates to a single source of truth in the tagger for pods and deployments.
Describe how to test/QA your changes
To QA, let's create a kind cluster with few nodes.
create a file called
kind-config.yaml
with the following content:Run the following command:
kind create cluster --config kind-config.yaml
Let's check and note the nodes labels:
Case 1: Ensure old configuration options still work as expected
Deploy the following with helm:
Create the following deployment:
Let's also add some labels and annotations to the default namespace:
Run
kubectl edit namespace default
, then add the following label and annotation:stale-annotation: some-old-annotation-value
stale-label: some-old-label-value
Verify that the tags are correct in the tagger for pods, namespaces and nodes, both on the cluster agent and on the node agent:
Verifying tags in DCA:
Verifying tags in DA:
Case 2: Ensure new configuration options work along with old configuration options
This time let's install the helm chart with both the old configuration options and new ones.
Notice how we have some conflicting configurations. For example,
kubernetes.io/os
label is mapped toos-as-tag
in the new configuration for nodes, but it is mapped toshould-not-be-used-should-be-overridden
in the oldnodeLabelsAsTags
configuration option. The new value should take precedence over the old one.If not already done, create the following deployment :
If not already done, let's also add some labels and annotations to the default namespace:
Run
kubectl edit namespace default
, then add the following label and annotation:stale-annotation: some-old-annotation-value
stale-label: some-old-label-value
Verify that the tags are correct in the tagger for deployments, pods, namespaces and nodes, both on the cluster agent and on the node agent:
Verifying tags in DCA:
Verifying tags in DA: