Allow for Metric Scraping interval to be defined overall & per-metric via YAML config #473

brandonh-msft · 2019-03-28T19:51:21Z

This PR accomplishes the following:

Move metric scraping schedule from Env variables -> YAML configuration under metricDefaults.scraping.schedule
Allow for an individual metric to have <metric>.scraping.schedule defined, overriding the value specified in metricDefaults
Require metricDefaults.scraping.schedule as part of validation
Adds a unit test to verify that validation fails if no default schedule is given in the YAML file
Reads the YAML config file at startup and spins up a MetricScrapingJob instance for every metric defined, with the correct schedule (inherited from default, overriden in metric def). I confirmed they all run in parallel on their correct schedules in my local testing 🎉

In addition to these features added, it changes the behavior of the Scraper to no longer require MetricDefaults as the MetricDeclarationProvider now supports an applyDefaults parameter on its Get() method (false by default; up for debate IMO) which will, when furnishing individual metrics, apply values in metricDefaults to any properties which don't have values in the metric itself. This isolates the scraper from needing to know how/when to do this.

…more

…don't push this logic down in to the scrapers

…rgeting different metrics to be scheduled

…as a cron expression instead of System.Timespan

…icDefaults to metrics that don't have values for those properties

…metrics to illustrate cusomizability and new usage of metricDefaults for overall scraping schedule (vs old env variable)

…don't push this logic down in to the scrapers

…as a cron expression instead of System.Timespan

…metrics to illustrate cusomizability and new usage of metricDefaults for overall scraping schedule (vs old env variable)

…der for UTs

promitor-bot · 2019-03-28T19:57:21Z

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:pr473

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr473 \
                         --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:pr473

promitor-bot · 2019-03-28T20:24:54Z

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:pr473

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr473 \
                         --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:pr473

brandonh-msft · 2019-03-28T20:27:12Z

forgot docs

promitor-bot · 2019-03-28T20:42:45Z

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:pr473

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr473 \
                         --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:pr473

brandonh-msft · 2019-03-28T20:52:12Z

anybody know why the snyk steps wouldn't be running for this right now?

cc @tomkerkhove

promitor-bot · 2019-03-28T20:57:48Z

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:pr473

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr473 \
                         --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:pr473

brusMX

Please consolidate the optional parameters, move the resourceGroupName to the same section that you just created

…on docs

…ation & validation

promitor-bot · 2019-03-28T21:31:25Z

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:pr473

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr473 \
                         --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:pr473

brusMX

LGTM

brandonh-msft · 2019-03-28T21:45:25Z

@tomkerkhove not sure why but we have 1 approving review and it still won't let us merge the PR. 🤷🏻‍♂️

brandonh-msft · 2019-03-28T22:34:46Z

@jsturtevant could you approve this and see if we can get "Squash & merge" to light up for us?

…impl

promitor-bot · 2019-03-29T16:16:15Z

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:pr473

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr473 \
                         --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:pr473

…impl

promitor-bot · 2019-03-29T16:26:55Z

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:pr473

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-scraper-pr473 \
                         --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:pr473

tomkerkhove · 2019-04-01T17:00:09Z

Thanks @brandonh-msft !

brandonh-msft added 21 commits March 28, 2019 11:33

Scraping Interval is not nullable on the metricDefaults in config any…

e09f68b

…more

Utilize the MetricsDeclarationProvider to apply defaults to metrics; …

796bda2

…don't push this logic down in to the scrapers

rename UseCronScheduler to ScheduleMetricScraping

80e10c5

Allow MetricScrapingJob to take the Metric to scrape; enables jobs ta…

6890bdf

…rgeting different metrics to be scheduled

reworking scraping.interval -> scraping.schedule (string) to be used …

6fd7aab

…as a cron expression instead of System.Timespan

adding scraper.schedule to metric builder used by UTs

ec92ce6

moving scheduling from env var -> YAML config

d3a5c74

add metric scraping schedule validation

2537e41

IEnumerable<T> + yield return refactor for Validators

817fdaa

when validating and returning metric detail, apply defaults from metr…

b134fa0

…icDefaults to metrics that don't have values for those properties

add default scraping schedule + different scraping schedule for some …

fa51f13

…metrics to illustrate cusomizability and new usage of metricDefaults for overall scraping schedule (vs old env variable)

Utilize the MetricsDeclarationProvider to apply defaults to metrics; …

11e121c

…don't push this logic down in to the scrapers

reworking scraping.interval -> scraping.schedule (string) to be used …

032c1bd

…as a cron expression instead of System.Timespan

adding scraper.schedule to metric builder used by UTs

f4c6f12

add default scraping schedule + different scraping schedule for some …

ec19652

…metrics to illustrate cusomizability and new usage of metricDefaults for overall scraping schedule (vs old env variable)

temp: working on UTs

0f33df7

fixing declaration provider's default assigning

7616c5c

deserializing scraping default w/ its own deserializer

c6b4240

adding validation to scraping default YAML item

a6ea912

adding scraping default value to default instance of declaration buil…

7179f49

…der for UTs

conforming Richard's changes to the new metricDefaults architecture

4695566

brandonh-msft requested review from jsturtevant and tomkerkhove as code owners March 28, 2019 19:51

codeFactor fix

e027b81

fixing resharper warnings

3d25c55

brandonh-msft closed this Mar 28, 2019

supporting docs for metric scraping interval change

ca0c378

brandonh-msft force-pushed the brandonh/feature/metric-scraping-interval-impl branch from 3d9aa3d to ca0c378 Compare March 28, 2019 20:54

brusMX suggested changes Mar 28, 2019

View reviewed changes

brandonh-msft added 4 commits March 28, 2019 14:07

move metric resourceGroupName -> new optional area of metric definiti…

a436d66

…on docs

this catch no longer needed due to internal refactoring of deserializ…

b91f750

…ation & validation

remove redundant setting of metricDefaults in UT declaration builder

f5e6de2

whoops

ef0d231

brandonh-msft requested a review from brusMX March 28, 2019 21:38

brusMX approved these changes Mar 28, 2019

View reviewed changes

Merge branch 'master' into brandonh/feature/metric-scraping-interval-…

a1acbb8

…impl

Merge branch 'master' into brandonh/feature/metric-scraping-interval-…

2019670

…impl

tomkerkhove merged commit 133a4ae into tomkerkhove:master Mar 29, 2019

lee0c mentioned this pull request Mar 29, 2019

Update helm chart to work with new scraping schedule value #476

Merged

brandonh-msft deleted the brandonh/feature/metric-scraping-interval-impl branch March 29, 2019 16:45

brandonh-msft mentioned this pull request Mar 29, 2019

Reintroduce fallback for scraping schedule? #477

Closed

tomkerkhove added this to the v1.0.0 milestone Apr 1, 2019

tomkerkhove unassigned brusMX Apr 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow for Metric Scraping interval to be defined overall & per-metric via YAML config #473

Allow for Metric Scraping interval to be defined overall & per-metric via YAML config #473

brandonh-msft commented Mar 28, 2019 •

edited

Loading

promitor-bot commented Mar 28, 2019

promitor-bot commented Mar 28, 2019

brandonh-msft commented Mar 28, 2019

promitor-bot commented Mar 28, 2019

brandonh-msft commented Mar 28, 2019

promitor-bot commented Mar 28, 2019

brusMX left a comment

promitor-bot commented Mar 28, 2019

brusMX left a comment

brandonh-msft commented Mar 28, 2019

brandonh-msft commented Mar 28, 2019

promitor-bot commented Mar 29, 2019

promitor-bot commented Mar 29, 2019

tomkerkhove commented Apr 1, 2019

Allow for Metric Scraping interval to be defined overall & per-metric via YAML config #473

Allow for Metric Scraping interval to be defined overall & per-metric via YAML config #473

Conversation

brandonh-msft commented Mar 28, 2019 • edited Loading

promitor-bot commented Mar 28, 2019

promitor-bot commented Mar 28, 2019

brandonh-msft commented Mar 28, 2019

promitor-bot commented Mar 28, 2019

brandonh-msft commented Mar 28, 2019

promitor-bot commented Mar 28, 2019

brusMX left a comment

Choose a reason for hiding this comment

promitor-bot commented Mar 28, 2019

brusMX left a comment

Choose a reason for hiding this comment

brandonh-msft commented Mar 28, 2019

brandonh-msft commented Mar 28, 2019

promitor-bot commented Mar 29, 2019

promitor-bot commented Mar 29, 2019

tomkerkhove commented Apr 1, 2019

brandonh-msft commented Mar 28, 2019 •

edited

Loading