Skip to content
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

[apm] Fix incorrect peer service config #16614

Merged
merged 2 commits into from
Apr 18, 2023
Merged

[apm] Fix incorrect peer service config #16614

merged 2 commits into from
Apr 18, 2023

Conversation

songy23
Copy link
Member

@songy23 songy23 commented Apr 14, 2023

What does this PR do?

Fix the inconsistent config name: core agent uses peer_service_stats_aggregation while trace agent expects peer_service_aggregation. Should use peer_service_aggregation in both places.

Motivation

Additional Notes

Possible Drawbacks / Trade-offs

Describe how to test/QA your changes

Test core agent config peer_service_aggregation is recognized by the trace agent.

Reviewer's Checklist

  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • Use the major_change label if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.
  • A release note has been added or the changelog/no-changelog label has been applied.
  • Changed code has automated tests for its functionality.
  • Adequate QA/testing plan information is provided if the qa/skip-qa label is not applied.
  • At least one team/.. label has been applied, indicating the team(s) that should QA this change.
  • If applicable, docs team has been notified or an issue has been opened on the documentation repo.
  • If applicable, the need-change/operator and need-change/helm labels have been applied.
  • If applicable, the k8s/<min-version> label, indicating the lowest Kubernetes version compatible with this feature.
  • If applicable, the config template has been updated.

@songy23 songy23 added this to the 7.46.0 milestone Apr 14, 2023
@songy23 songy23 requested review from djova and jdgumz April 14, 2023 20:16
@songy23 songy23 requested review from a team as code owners April 14, 2023 20:16
@songy23 songy23 added the team/agent-apm trace-agent label Apr 14, 2023
Copy link
Contributor

@buraizu buraizu left a comment

Choose a reason for hiding this comment

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

Left a small update to the release note per our style guide, but otherwise LGTM.

releasenotes/notes/fix-peer-service-config-2ecb186539.yaml Outdated Show resolved Hide resolved
@pr-commenter
Copy link

pr-commenter bot commented Apr 14, 2023

Bloop Bleep... Dogbot Here

Regression Detector Results

Run ID: 45ddc14a-28f0-47d8-abd3-bf79bfc53b2e
Baseline: da97601
Comparison: c9c1df0
Total datadog-agent CPUs: 7

Explanation

A regression test is an integrated performance test for datadog- agent in a repeatable rig, with varying configuration for datadog- agent. What follows is a statistical summary of a brief datadog- agent run for each configuration across SHAs given above. The goal of these tests are to determine quickly if datadog-agent performance is changed and to what degree by a pull request.

The table below, if present, lists those experiments that have experienced a statistically significant change in mean optimization goal performance between baseline and comparison SHAs with 90.00% confidence OR have been detected as newly erratic. Negative values mean that baseline is faster, positive comparison. Results that do not exhibit more than a ±5.00% change in their mean optimization goal are discarded. An experiment is erratic if its coefficient of variation is greater than 0.1. The abbreviated table will be omitted if no interesting change is observed.

Changes in experiment optimization goals with confidence ≥ 90.00% and |Δ mean %| ≥ 5.00%:

experiment goal Δ mean Δ mean % confidence
tcp_dd_logs_filter_exclude ingress throughput 7.38MiB/CPU-s 5.07 100.00%
Fine details of change detection per experiment.
experiment goal Δ mean Δ mean % confidence baseline mean baseline stdev baseline stderr baseline outlier % baseline CoV comparison mean comparison stdev comparison stderr comparison outlier % comparison CoV erratic declared erratic
tcp_dd_logs_filter_exclude ingress throughput 7.38MiB/CPU-s 5.07 100.00% 145.76MiB/CPU-s 13.3MiB/CPU-s 174.5KiB/CPU-s 0.0 0.09123 153.14MiB/CPU-s 11.74MiB/CPU-s 154.03KiB/CPU-s 0.0 0.076646 False False
uds_dogstatsd_to_api ingress throughput 2.32KiB/CPU-s 3.08 99.98% 75.16KiB/CPU-s 34.82KiB/CPU-s 457.37B/CPU-s 0.0 0.463236 77.48KiB/CPU-s 34.89KiB/CPU-s 458.36B/CPU-s 0.0 0.450243 True False
file_to_blackhole egress throughput 12.53B/CPU-s 1.23 99.46% 1021.97B/CPU-s 89.5B/CPU-s 3.08B/CPU-s 0.0 0.087523 1.01KiB/CPU-s 95.51B/CPU-s 3.28B/CPU-s 0.0 0.09227 False False
tcp_syslog_to_blackhole ingress throughput 10.15KiB/CPU-s 0.14 90.34% 7.12MiB/CPU-s 304.53KiB/CPU-s 3.93KiB/CPU-s 0.0 0.04177 7.13MiB/CPU-s 362.88KiB/CPU-s 4.68KiB/CPU-s 0.0 0.049704 False False
otel_to_otel_logs ingress throughput -2.62MiB/CPU-s -0.33 100.00% 783.34MiB/CPU-s 30.75MiB/CPU-s 406.3KiB/CPU-s 0.0 0.039257 780.72MiB/CPU-s 34.15MiB/CPU-s 451.05KiB/CPU-s 0.0 0.043732 False False

Copy link
Contributor

Choose a reason for hiding this comment

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

We should rather merge this PR as bug fix in 7.45 for #16103 and not include a release note at all.
Since we're still in QA phase, we shouldn't release an agent version with the wrong config name peer_service_stats_aggregation.
@songy23 please fix the CI, remove the release note and try to get the PR merged ASAP, I will change the milestone.

Copy link
Member Author

@songy23 songy23 Apr 18, 2023

Choose a reason for hiding this comment

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

Done, thanks for the clarification @ahmed-mez can you approve the PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Please ask Kacper to unfreeze the PR in #agent-release-sync

@ahmed-mez ahmed-mez modified the milestones: 7.46.0, 7.45.0 Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants