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

[DAQ-4166] Added option to include projects #545

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Dawid-Kaszubski
Copy link
Contributor

A copy of the community pull request: #543
A Dynatrace developer responsible for this repository copied and pushed changes to allow the builds to pass. @ajth13 was consulted on the copy and creation of this PR. Thank you once again for your input in the dynatrace-gcp-monitor development.

Original PR description:

Where you can currently only exclude projects from monitoring this change adds the ability to specify a list to include.
Business Justification

Enterprise customers with a large number of Dynatrace tenants and wants to be able to easily control which metrics are sent to which tenant and include list is much smaller than excluding all projects from other Dynatrace tenants.

Considerations

The project must still be included in the metric scoping this only filters the list of scoped projects.
The exclude logic is preferred so if a project is in both lists it will not be included.

@@ -175,6 +175,10 @@ scopingProjectSupportEnabled: "false"
excludedProjects: ""
# excludedProjectsByPrefix: comma separated list of projects substring that will be excluded from monitoring (e.g. "project-a,proj,pro").
excludedProjectsByPrefix: ""
# includedProjects: comma separated list of projects that will be included from monitoring (e.g. "project-a,project-b,project-c").
includedProjects: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say there's no need for this, as long as we have "includedProjectsByPrefix".
The thing is we have both for exclusion too, but that was because we firstly had "excludedProjects" and then some customer asked for excludedProjectsByPrefix". IMHO, we could stick only with the "...ByPrefix" ones.
But for the exclusion, removing one now means that some customers need to switch if they want to redeploy a new version.
To consider, at least.

Copy link

Choose a reason for hiding this comment

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

Yeah totally reasonable suggestion. I added both only to replicate the existing behavior (but inverted) but there is not really any need as you pointed out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. So, to me, it's fine either way. We can leave it as it is or clean it up a bit, with some note in the readme or somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants