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

fix iis.*_up service check when configured using a mapping #13747

Merged
merged 1 commit into from
Mar 15, 2023

Conversation

ian28223
Copy link
Contributor

@ian28223 ian28223 commented Jan 20, 2023

What does this PR do?

Prevents iis.*_up ServiceChecks with site:/app_pool: = include/exclude - when used in below example configuration.

 -  sites:
      include:
      - Default Web Site
      - foo
      - bar
    app_pools:
      include_fast:
      - baz

Motivation

  • AGENT-9272

Additional Notes

This only addresses the include/exclude tags when a mapping is provided but does not take into consideration if regex patterns are provided.

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.

@ian28223 ian28223 requested review from a team as code owners January 20, 2023 06:36
@ian28223 ian28223 force-pushed the ian.bucad/iis_servicecheck branch 2 times, most recently from 9db6a91 to 50e37e9 Compare January 20, 2023 10:43
@ian28223 ian28223 force-pushed the ian.bucad/iis_servicecheck branch from 50e37e9 to 5008d22 Compare January 20, 2023 10:48
@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Merging #13747 (5008d22) into master (7072f59) will increase coverage by 0.17%.
The diff coverage is 100.00%.

Flag Coverage Δ
iis 94.64% <100.00%> (+38.81%) ⬆️

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

@clarkb7
Copy link
Contributor

clarkb7 commented Jan 23, 2023

I think this is going to require more fixes than just the type. The dict config format accepts regular expressions to match, but CompatibilityPerfObject treats it like a list of instance names.
I think if you configure the following you'll get a service check for Default*.

 -  sites:
      include:
      - Default*

@ian28223
Copy link
Contributor Author

ian28223 commented Jan 27, 2023

I think this is going to require more fixes than just the type. The dict config format accepts regular expressions to match, but CompatibilityPerfObject treats it like a list of instance names. I think if you configure the following you'll get a service check for Default*.

 -  sites:
      include:
      - Default*

You're right. This does not take into consideration if regex were provided. However, fixing that I think should be another PR since the issue you describe remains even if sites/app_pools are given a list. E.g.

 -  sites:
      - Default*

This PR for now only prevents the dict keys (include,exclude) from being treated as a site or app_pool

@iglendd iglendd merged commit cf46a63 into master Mar 15, 2023
@iglendd iglendd deleted the ian.bucad/iis_servicecheck branch March 15, 2023 12:26
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.

4 participants