-
Notifications
You must be signed in to change notification settings - Fork 156
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
Add Support for Managing Service Level Objectives #250
Conversation
sync master
5f045d6
to
6d05215
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks really good, but I left some comments that need to be addressed inline.
The test files were in a different package to make sure they do black-box testing (e.g. that none of the internal structures are necessary when actually using the library). I guess I'd prefer keeping things as they are, as this is also completely unrelated to adding SLOs and mixes 2 different features in a single commit.
0ead720
to
6804942
Compare
Context for schemas at the simplest validation level (jsonschema): Create Schema: {
"type": "object",
"properties": {
"type": {"type": "string", "enum": ["metric", "monitor"]},
"name": {"type": "string", "minLength": 1, "maxLength": 16384},
"monitor_ids": {
"type": "array",
"maxItems": 100,
"uniqueItems": true,
"items": {"type": "number", "exclusiveMinimum": 0},
},
"monitor_search": {"type": "string", "minLength": 1, "maxLength": 16384},
"query": {
"type": "object",
"properties": {
"numerator": {"type": "string", "minLength": 1, "maxLength": 65536},
"denominator": {"type": "string", "minLength": 1, "maxLength": 65536},
},
},
"tags": {
"type": "array",
"maxItems": 128,
"uniqueItems": true,
"items": {"type": "string", "minLength": 1, "maxLength": 256},
},
"thresholds": {
"type": "object",
"patternProperties": {
"^[0-9]+[a-z]+$": {
"type": "object",
"properties": {
"slo": {"type": "number", "minimum": 0, "maximum": 100},
"warning": {"type": "number", "minimum": 0, "maximum": 100},
},
"required": ["slo"],
}
},
},
},
"required": ["type", "name", "thresholds"],
} Update Schema: {
"type": "object",
"properties": {
"type": {"type": "string", "enum": ["metric", "monitor"]},
"name": {"type": "string", "minLength": 1, "maxLength": 16384},
"monitor_ids": {
"type": "array",
"maxItems": 100,
"uniqueItems": true,
"items": {"type": "number", "exclusiveMinimum": 0},
},
"monitor_search": {"type": "string", "minLength": 1, "maxLength": 16384},
"query": {
"type": "object",
"properties": {
"numerator": {"type": "string", "minLength": 1, "maxLength": 65536},
"denominator": {"type": "string", "minLength": 1, "maxLength": 65536},
},
},
"tags": {
"type": "array",
"maxItems": 128,
"uniqueItems": true,
"items": {"type": "string", "minLength": 1, "maxLength": 256},
},
"thresholds": {
"type": "object",
"patternProperties": {
"^[0-9]+[a-z]+$": {
"type": "object",
"properties": {
"slo": {"type": "number", "minimum": 0, "maximum": 100},
"warning": {"type": "number", "minimum": 0, "maximum": 100},
},
"required": ["slo"],
}
},
},
},
"required": ["type"],
} ids_schema: {
"type": "array",
"minItems": 1,
"maxItems": 1000,
"uniqueItems": true,
"items": {"type": "string", "minLength": 32, "maxLength": 32},
} |
ab6e70f
to
4174cfa
Compare
update some test files to use the same package for easier dev TODO: the /v1/slo endpoint does not yet have the status endpoint defined yet, but will be coming in another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, thanks for your work!
Adds support for Service Level Objectives
/v1/slo
endpoints.Note: This does not include the status API currently, that is a work in flight and will come with a separate PR.
Also updates some packages from
datadog_test
todatadog
. I'm not sure why these were different in the first place