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.

Sign up for GitHub

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

Jump to bottom

Support for multiple resource groups #442

Merged
merged 12 commits into from
Mar 26, 2019
Merged

Support for multiple resource groups #442

merged 12 commits into from
Mar 26, 2019

Conversation

brusMX
Copy link
Contributor

@brusMX brusMX commented Mar 26, 2019

Fixes #294

Proposed Changes

  • Refactoring Scraping to be done per Resource Group per metric

@promitor-bot
Copy link

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr442

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr442 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env PROMITOR_AUTH_APPKEY='<azure-ad-app-key>' \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr442

@tomkerkhove tomkerkhove modified the milestones: v1.1.0, v1.0.0 Mar 26, 2019
@tomkerkhove tomkerkhove marked this pull request as ready for review March 26, 2019 20:03
@tomkerkhove tomkerkhove self-requested a review as a code owner March 26, 2019 20:03
tomkerkhove
tomkerkhove reviewed Mar 26, 2019
View reviewed changes
Copy link
Owner

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

Looks good, added a few suggestions and will wait on the docs

src/Promitor.Core.Scraping/Configuration/Model/Metrics/MetricDefinition.cs Outdated Show resolved Hide resolved
src/Promitor.Core.Scraping/Scraper.cs Outdated Show resolved Hide resolved
...lization/MetricsDeclaration/MetricsDeclarationWithAzureStorageQueueYamlSerializationTests.cs Outdated Show resolved Hide resolved
tomkerkhove
tomkerkhove requested changes Mar 26, 2019
View reviewed changes
Copy link
Owner

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

This looks good to me, some minor changes but would like to have them all before we merge.

Also how about adding Support for multi-resource group scraping to the features on the root of the docs?

docs/configuration/metrics/index.md Outdated Show resolved Hide resolved
@brusMX brusMX force-pushed the multi-resource-group branch from cd549db to 9d3c7a4 Compare March 26, 2019 21:38
@brusMX brusMX force-pushed the multi-resource-group branch from 9d3c7a4 to b183222 Compare March 26, 2019 21:42
@promitor-bot
Copy link

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr442

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr442 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env PROMITOR_AUTH_APPKEY='<azure-ad-app-key>' \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr442

tomkerkhove
tomkerkhove requested changes Mar 26, 2019
View reviewed changes
Copy link
Owner

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

Need to re-review, you use rebase!

@promitor-bot
Copy link

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr442

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr442 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env PROMITOR_AUTH_APPKEY='<azure-ad-app-key>' \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr442

tomkerkhove and others added 2 commits March 26, 2019 15:32
@tomkerkhove @brusMX
Update comments
40bb18f 
Co-Authored-By: brusMX <[email protected]>
@tomkerkhove @brusMX
Typo Scraper
eb75d44 
Co-Authored-By: brusMX <[email protected]>
tomkerkhove
tomkerkhove approved these changes Mar 26, 2019
View reviewed changes
docs/configuration/metrics/index.md Outdated Show resolved Hide resolved
@promitor-bot
Copy link

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr442

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr442 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env PROMITOR_AUTH_APPKEY='<azure-ad-app-key>' \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr442

1 similar comment
@promitor-bot
Copy link

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr442

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr442 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env PROMITOR_AUTH_APPKEY='<azure-ad-app-key>' \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr442

@promitor-bot
Copy link

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr442

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr442 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env PROMITOR_AUTH_APPKEY='<azure-ad-app-key>' \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr442

@promitor-bot
Copy link

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr442

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr442 \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env PROMITOR_AUTH_APPKEY='<azure-ad-app-key>' \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr442

@brusMX brusMX merged commit f88e347 into tomkerkhove:master Mar 26, 2019
@brusMX brusMX deleted the multi-resource-group branch March 26, 2019 22:56
@adamconnelly adamconnelly mentioned this pull request Aug 10, 2019
Closed
@adamconnelly adamconnelly mentioned this pull request Aug 22, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomkerkhove tomkerkhove tomkerkhove approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@brusMX @promitor-bot @tomkerkhove