Open jaredcnance opened 4 years ago
heitorlessa
commented
4 years ago I'd vote for raising a ValueError since CloudWatch does not create a metric if Namespace, Dimension, and Metric - Unless that's not the case with EMF...
I agree it can crash an application/function. However, it can be equally frustrating, or damaging depending on the use case, if metrics aren't being created if one forgot a namespace/dimension.
heitorlessa
commented
4 years ago UPDATE - Updated JSON Schema to require at least one Metric. Discovered while writing tests that I could create an EMF object without a metric
@jaredcnance I noticed the JSON Schema in the docs is wrong, what's the best way to suggest a fix?
Here's a modified version that accounts for:
Name is a MUST{
"type": "object",
"title": "Root Node",
"required": [
"_aws"
],
"properties": {
"_aws": {
"$id": "#/properties/_aws",
"type": "object",
"title": "Metadata",
"required": [
"Timestamp",
"CloudWatchMetrics"
],
"properties": {
"Timestamp": {
"$id": "#/properties/_aws/properties/Timestamp",
"type": "integer",
"title": "The Timestamp Schema",
"examples": [
1565375354953
]
},
"CloudWatchMetrics": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics",
"type": "array",
"title": "MetricDirectives",
"items": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics/items",
"type": "object",
"title": "MetricDirective",
"required": [
"Namespace",
"Dimensions",
"Metrics"
],
"properties": {
"Namespace": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics/items/properties/Namespace",
"type": "string",
"title": "CloudWatch Metrics Namespace",
"examples": [
"MyApp"
],
"pattern": "^(.*)$",
"minLength": 1
},
"Dimensions": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics/items/properties/Dimensions",
"type": "array",
"title": "The Dimensions Schema",
"minItems": 1,
"items": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics/items/properties/Dimensions/items",
"type": "array",
"title": "DimensionSet",
"minItems": 1,
"maxItems": 9,
"items": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics/items/properties/Dimensions/items/items",
"type": "string",
"title": "DimensionReference",
"examples": [
"Operation"
],
"pattern": "^(.*)$",
"minItems": 1
}
}
},
"Metrics": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics/items/properties/Metrics",
"type": "array",
"title": "MetricDefinitions",
"minItems": 1,
"items": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics/items/properties/Metrics/items",
"type": "object",
"title": "MetricDefinition",
"required": [
"Name"
],
"minItems": 1,
"properties": {
"Name": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics/items/properties/Metrics/items/properties/Name",
"type": "string",
"title": "MetricName",
"examples": [
"ProcessingLatency"
],
"pattern": "^(.*)$",
"minLength": 1
},
"Unit": {
"$id": "#/properties/_aws/properties/CloudWatchMetrics/items/properties/Metrics/items/properties/Unit",
"type": "string",
"title": "MetricUnit",
"examples": [
"Milliseconds"
],
"pattern": "^(Seconds|Microseconds|Milliseconds|Bytes|Kilobytes|Megabytes|Gigabytes|Terabytes|Bits|Kilobits|Megabits|Gigabits|Terabits|Percent|Count|Bytes\\/Second|Kilobytes\\/Second|Megabytes\\/Second|Gigabytes\\/Second|Terabytes\\/Second|Bits\\/Second|Kilobits\\/Second|Megabits\\/Second|Gigabits\\/Second|Terabits\\/Second|Count\\/Second|None)$"
}
}
}
}
}
}
}
}
}
}
}
jaredcnance
commented
4 years ago Thanks @heitorlessa! I'll get the docs updated.
Problem
There are a few places within the package where logical/configuration errors are swallowed silently (see #19 for one example). We should add a configuration parameter that allows developers to throw validation exceptions in these cases. The original reason for not throwing exceptions was the idea that a monitoring tool shouldn't negatively impact the application. It would be possible that throwing exceptions in these cases would cause applications to crash at runtime. However, this should be weighed against the behavior today which arguably violates the principle of least astonishment. The current behavior is not clear from the interface design.
Next Steps
THROW_ON_VALIDATION_ERRORS(or better name?) that defaults to false.