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

[windows][usm] Fix json parsing for usm #30780

Merged
merged 2 commits into from
Nov 6, 2024
Merged

Conversation

derekwbrown
Copy link
Contributor

The parsing of datadog.json for USM service tagging was far too restrictive. It assumed all values were always string values, and failed if any entry had a non-string (e.g. bool) value; even though those values are allowed for other keys.

Fixes parser to allow this case, and also updates test to have the additional entries in them.

Motivation

Customer reported issue with USM service tagging not working

Describe how to test/QA your changes

Automated tests in place.

Create a datadog.json in the director

Install IIS.
Create one or more IIS sites in the IIS configuration.

Place a datadog.json or web.config file in the root of the site directory, with any combination of env/version/service tags.
The file formats are documented
https://datadoghq.atlassian.net/wiki/spaces/WKIT/pages/4012835378/USM+APM+Compatibility#.NET-Tracer-configuration-file-formats

In addition, the customer had a valid entry that we were parsing as invalid, such as:
"DD_RUNTIME_METRICS_ENABLED": true

Add this entry to the top level block.

Make a connection to the site.

YOu can check that the appropriate tag(s) have been added by hitting the HTTP information endpoint:

(from the source tree, download NamedPipeCmd.ex)
.\NamedPipeCmd.exe -method GET -path /network_tracer/debug/http_monitoring -quiet |convertfrom-json

YOu should see output such as:

Client : @{IP=::1; Port=80}
Server : @{IP=::1; Port=62700}
DNS :
Path : /
Method : GET
ByStatus : @{200=}
StaticTags : 0
DynamicTags : {http.iis.app_pool:DefaultAppPool, http.iis.site:1, http.iis.sitename:Default Web Site}
with the addition of service, env, and or version tags that match what was configured in the file that you applied.

Possible Drawbacks / Trade-offs

The parsing of `datadog.json` for USM service tagging was far too restrictive.
It assumed all values were always string values, and failed if any entry
had a non-string (e.g. bool) value; even though those values are allowed for other keys.

Fixes parser to allow this case, and also updates test to have the additional entries
in them.
@derekwbrown derekwbrown added this to the 7.60.0 milestone Nov 5, 2024
@derekwbrown derekwbrown requested a review from a team as a code owner November 5, 2024 17:39
@github-actions github-actions bot added the medium review PR review might take time label Nov 5, 2024
@agent-platform-auto-pr
Copy link
Contributor

agent-platform-auto-pr bot commented Nov 5, 2024

[Fast Unit Tests Report]

On pipeline 48376905 (CI Visibility). The following jobs did not run any unit tests:

Jobs:
  • tests_deb-arm64-py3
  • tests_deb-x64-py3
  • tests_flavor_dogstatsd_deb-x64
  • tests_flavor_heroku_deb-x64
  • tests_flavor_iot_deb-x64
  • tests_rpm-arm64-py3
  • tests_rpm-x64-py3

If you modified Go files and expected unit tests to run in these jobs, please double check the job logs. If you think tests should have been executed reach out to #agent-devx-help

@agent-platform-auto-pr
Copy link
Contributor

agent-platform-auto-pr bot commented Nov 5, 2024

Test changes on VM

Use this command from test-infra-definitions to manually test this PR changes on a VM:

inv create-vm --pipeline-id=48376905 --os-family=ubuntu

Note: This applies to commit 07eaf9b

@derekwbrown derekwbrown added the backport/7.60.x Automatically create a backport PR to 7.60.x label Nov 6, 2024
@derekwbrown
Copy link
Contributor Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Nov 6, 2024

🚂 MergeQueue: pull request added to the queue

The median merge time in main is 23m.

Use /merge -c to cancel this operation!

@dd-mergequeue dd-mergequeue bot merged commit 1c2a21b into main Nov 6, 2024
212 of 213 checks passed
@dd-mergequeue dd-mergequeue bot deleted the db/fix-json-parsing-usm branch November 6, 2024 22:25
@github-actions github-actions bot modified the milestones: 7.60.0, 7.61.0 Nov 6, 2024
agent-platform-auto-pr bot pushed a commit that referenced this pull request Nov 6, 2024
@derekwbrown derekwbrown added the qa/done QA done before merge and regressions are covered by tests label Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/7.60.x Automatically create a backport PR to 7.60.x changelog/no-changelog medium review PR review might take time qa/done QA done before merge and regressions are covered by tests team/usm The USM team team/windows-kernel-integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants