Disable server info and version collection when collect_server_info is false #14610

HadhemiDD · 2023-05-25T15:53:37Z

What does this PR do?

When the collect_server_info is set to false, we shouldn't try to collect the server metadata, including the server version.

Motivation

A customer complained about errors when attempting to collect server info and version, despite them setting the collect_server_info to false.
The log they see:
(pkg/collector/python/datadog_agent.go:134 in LogMessage) | envoy:9e356679f384318e | (utils.py:55) | Envoy endpoint <endpoint>/server_info responded with HTTP status code 404
Jira card

Additional Notes

This was originally done here in this PR-9298 but later the raw_version was updated as part of this PR-10752 to make calls to the function generating the above log: _get_server_info

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
PR title must be written as a CHANGELOG entry (see why)
Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
PR must have changelog/ and integration/ labels attached
If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

github-actions · 2023-05-25T16:04:35Z

Test Results

    6 files     6 suites 2m 40s ⏱️
  87 tests   85 ✔️   2 💤 0 ❌
264 runs 240 ✔️ 24 💤 0 ❌

Results for commit 247cd6c.

♻️ This comment has been updated with latest results.

codecov · 2023-05-25T16:05:38Z

Codecov Report

Merging #14610 (247cd6c) into master (e8116f1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Flag	Coverage Δ
envoy	`95.30% <100.00%> (+0.69%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

github-actions · 2023-05-30T07:34:39Z

Label changelog/Changed was added to this Pull Request, so the next release will bump major version. Please make sure this is a breaking change, or use the changelog/Fixed label.

yzhan289

Thanks for adding this for envoy OpenMetricsV2! Could you also add some tests for this?

envoy/datadog_checks/envoy/envoy.py

…n collection

…info before attempting to collect server info, even when the base url is well formated

…is false

FlorentClarret

Looks good to me thanks for that! 🙇

Just waiting for Andrew to have a quick look

yzhan289

LGTM! Thanks for this fix 🙇

FlorentClarret · 2023-06-20T14:37:14Z

remove lines Update CHANGELOG.md Draft CHANGELOG.md Update metadata.csv Update metadata.json Update conftest.py Update CHANGELOG.md Remove change log Update ecs_fargate.py Add __init__ function Fix ecs_fargate.py and add tests Add new performance counter metrics (#14625) * Add new cluster metrics * Add host and VM metrics * Add realtime fixtures * update metadata.csv * Add throughput metric Move cancel waiting logic to test functions for DBMAsyncJob (#14773) * init commit * revert changes to sqlserver, mysql, utils tests Add validations for removed dependencies (#14556) * Map out new licenses validation * Implement validations for extra licenses * Add constants to config.toml * Implement license validation * Uncomment legacy licenses validation * Keep license command addition in same place * Small style change * Update config.toml override values * Refactor * Fix style * Apply suggestions from code review Co-authored-by: Ofek Lev <ofekmeister@gmail.com> * Update suggestions * Require CI for license validation tests and update to use empty envvars * Fix permission for file * Add windows version of setting github env vars * Fix windows file * Change to powershell script * Output GITHUB_ENV on windows CI * Convert entirely to powershell * Change back to bat file * Test DD_GITHUB_USER value * Print github user in license test * Manually set Github user and token in test * Fix config_file * Print github user * Check if tokens are the same * Remove additional space in bat script * Fix style and remove test code * Change order of scripts * Try commenting out model.github override * Revert previous commit * Change to threads instead of async * Switch out async request to requests * Clean up * Fix style --------- Co-authored-by: Ofek Lev <ofekmeister@gmail.com> DOCS-5656 gke setup links to operator/helm (#14746) dbm-oracle-dashboard (#14736) Internally compile the `include` patterns in the autodiscovery feature (#14768) * Internally compile the `include` patterns in the discovery feature * address Use Git for versioning (#14778) Upgrade Pydantic model code generator (#14779) * Upgrade Pydantic model code generator * address build standalone binaries for ddev (#14774) Remove `pyperclip` dependency and clipboard functionality (#14782) [Release] Bumped datadog_checks_dev version to 20.0.0 (#14784) * [Release] Bumped datadog_checks_dev version to 20.0.0 * [Release] Update metadata Bump the minimum version of datadog-checks-dev (#14785) update license path (#14783) Allow all projects to be collected in REST implementation (#14433) * Bug Fix Teamcity rest with all projects * . Rewrite Postgres size query and add `postgresql.relation.{tuples,pages,all_visible}` + toast_size metrics (#14500) * Bumped dependency version * Rewrite size metric query Use new query executor Split toast size from table size Add partition_of tag Add pages/tuples/allvisible metrics Optimise query to minimise stat calls * Add partitioned test tables * Update metadata with new metric * Fix version check * Fix wal_level for tests [SNMP] Add metadata for traps telemetry metrics (#14769) * Add metadata for traps telemetry metrics * Remove commas from desc 🤦‍ * Add units Temporarily disable py2 tests on PRs (#14793) fix(redisdb): return len of stream instead of 1 (#14722) Currently the code compute the len of the stream but report always 1 This change fixes this. Update Netflow dashboard (#14794) * update Netflow dashboard * remove datadog_demo_keep:true * avg -> sum * rename dashboard revert manifest.json (#14797) Add User Profiles support (#14752) Remove Content (#14766) Update wording and add extra install directions for ODBC (#14781) * Update wording and add extra install directions for ODBC * Update README.md Add ability to choose tag to append to VM hostname (#14657) * Add ability to choose tag to append to VM hostname * Add a test for integration tags * Sort list * Change log to debug * Fix style * Allow user to choose a datadog tag for vm hostname Fix ability to release ddev (#14790) Disable server info and version collection when collect_server_info is false (#14610) * if collect_server_info is set to false disable server info and version collection * the collect metadata in check.py has to check for the collect_server_info before attempting to collect server info, even when the base url is well formated * add testing to see if metadata is collected when collect_server_info is false * add testing to see if metadata is collected when collect_server_info is false * fix typo * fix typo * commit * commit * commit * commit typo * fix check.py [Release] Bumped datadog_checks_dev version to 20.0.1 (#14806) * [Release] Bumped datadog_checks_dev version to 20.0.1 * [Release] Update metadata [Release] Bumped ddev version to 3.0.0 (#14807) * [Release] Bumped ddev version to 3.0.0 * [Release] Update metadata fix build flake for ddev (#14808) Fix ddev platform installers and releasing (#14812) Bump postgres integration to Python 3 (#14813) update changelog generation (#14810) Update ecs_fargate/tests/fixtures/metadata_v4.json Co-authored-by: Cedric Lamoriniere <cedric.lamoriniere@datadoghq.com> Update ecs_fargate/tests/fixtures/stats_linux_v4.json Co-authored-by: Cedric Lamoriniere <cedric.lamoriniere@datadoghq.com> Update test_unit_v4.py Update license term Update test_unit_v4.py update license format Update metadata.csv Fix unit name Set the `marker` option to `not e2e` by default (#14804) Add profile for hp-ilo (#14771) * add profile for hp-ilo * add tests for new hp-ilo profile * fix linter * hp-ilo4 extends hp-ilo * delete unnecessary product_name field * lint * move hp-ilo to default-profiles Update profiles with missing devices (#14695) * update cisco-asr * update cisco-catalyst-wlc * update cisco-catalyst * update cisco-legacy-wlc * update cisco-nexus * update dell-poweredge * update juniper-ex * update juniper-mx * add cisco-isr * add models + move cisco5700WLC to cisco-catalyst-wlc * move cisco-isr to default-profiles Add profile 3com-huawei (#14694) Revert "Set the `marker` option to `not e2e` by default (#14804)" (#14815) This reverts commit 3f4c885. Sort assert_device_metadata tags (#14816) Add per vendor generic profiles (#14721) * add dell generic profile * add fortinet generic profile * add juniper generic profile * move vendor profiles to default-profiles * add test for cisco * add test for dell * add test for fortinet * add test for juniper * linter * linter Update formatting for changelogs (#14814) * Update formatting for changelogs * Update formatting for changelogs

HadhemiDD added the integration/envoy label May 25, 2023

HadhemiDD self-assigned this May 25, 2023

HadhemiDD added the changelog/no-changelog label May 25, 2023

HadhemiDD added changelog/Changed and removed changelog/no-changelog labels May 30, 2023

HadhemiDD marked this pull request as ready for review May 30, 2023 12:00

HadhemiDD requested a review from a team as a code owner May 30, 2023 12:00

HadhemiDD added changelog/Fixed and removed changelog/Changed labels May 30, 2023

yzhan289 requested changes May 30, 2023

View reviewed changes

envoy/datadog_checks/envoy/envoy.py Outdated Show resolved Hide resolved

HadhemiDD force-pushed the hadhemi-envoy branch 2 times, most recently from 81e03bf to 8996405 Compare June 13, 2023 10:34

HadhemiDD force-pushed the hadhemi-envoy branch from 2f2ab9b to 8f032a6 Compare June 13, 2023 15:42

HadhemiDD added 11 commits June 19, 2023 11:39

if collect_server_info is set to false disable server info and versio…

4f95d38

…n collection

the collect metadata in check.py has to check for the collect_server_…

3eb4ba1

…info before attempting to collect server info, even when the base url is well formated

add testing to see if metadata is collected when collect_server_info …

59e94f0

…is false

add testing to see if metadata is collected when collect_server_info …

456e3d5

…is false

fix typo

337d469

fix typo

490e2a2

commit

222e94f

commit

76759a9

commit

5a7a969

commit typo

4f36699

fix check.py

247cd6c

HadhemiDD force-pushed the hadhemi-envoy branch from 8557e4e to 247cd6c Compare June 19, 2023 09:40

HadhemiDD modified the milestone: 7.45 Jun 19, 2023

FlorentClarret approved these changes Jun 19, 2023

View reviewed changes

FlorentClarret added this to the 7.47 milestone Jun 19, 2023

FlorentClarret requested a review from yzhan289 June 19, 2023 13:30

yzhan289 approved these changes Jun 20, 2023

View reviewed changes

FlorentClarret merged commit b8c3ee4 into master Jun 20, 2023
34 checks passed

FlorentClarret deleted the hadhemi-envoy branch June 20, 2023 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable server info and version collection when collect_server_info is false #14610

Disable server info and version collection when collect_server_info is false #14610

HadhemiDD commented May 25, 2023 •

edited

Loading

github-actions bot commented May 25, 2023 •

edited

Loading

codecov bot commented May 25, 2023 •

edited

Loading

github-actions bot commented May 30, 2023

yzhan289 left a comment

FlorentClarret left a comment

yzhan289 left a comment

FlorentClarret commented Jun 20, 2023 •

edited

Loading

Disable server info and version collection when collect_server_info is false #14610

Disable server info and version collection when collect_server_info is false #14610

Conversation

HadhemiDD commented May 25, 2023 • edited Loading

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

github-actions bot commented May 25, 2023 • edited Loading

Test Results

codecov bot commented May 25, 2023 • edited Loading

Codecov Report

github-actions bot commented May 30, 2023

yzhan289 left a comment

Choose a reason for hiding this comment

FlorentClarret left a comment

Choose a reason for hiding this comment

yzhan289 left a comment

Choose a reason for hiding this comment

FlorentClarret commented Jun 20, 2023 • edited Loading

HadhemiDD commented May 25, 2023 •

edited

Loading

github-actions bot commented May 25, 2023 •

edited

Loading

codecov bot commented May 25, 2023 •

edited

Loading

FlorentClarret commented Jun 20, 2023 •

edited

Loading