Skip to content

Commit 9c62df6

Browse files
authored
include tags in validation result responses (Netflix#1203)
Metrics are typically submitted in large batches and without this context it can be difficult for users to narrow it down to particular data points that are problematic. Sample of updated response: ```json { "type": "partial", "errorCount": 1, "message": [ "invalid characters in value: name = [answerTo Everything] ([--.0-9A-Z^_a-z~] are allowed) (tags={\"nf.node\":\"i-123\",\"atlas.dstype\":\"gauge\",\"name\":\"answerTo Everything\"})" ] } ``` Fixes Netflix#1199
1 parent dd30848 commit 9c62df6

24 files changed

+153
-86
lines changed

atlas-core/src/main/scala/com/netflix/atlas/core/validation/CompositeTagRule.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,18 @@ package com.netflix.atlas.core.validation
1818
/** Verifies that all of the tag rules match. */
1919
case class CompositeTagRule(tagRules: List[TagRule]) extends TagRule {
2020

21-
override def validate(k: String, v: String): ValidationResult = {
21+
override def validate(k: String, v: String): String = {
2222
validate(tagRules, k, v)
2323
}
2424

2525
@scala.annotation.tailrec
26-
private def validate(rules: List[TagRule], k: String, v: String): ValidationResult = {
26+
private def validate(rules: List[TagRule], k: String, v: String): String = {
2727
if (rules.isEmpty) {
28-
ValidationResult.Pass
28+
TagRule.Pass
2929
} else {
3030
val r = rules.head
3131
val result = r.validate(k, v)
32-
if (result == ValidationResult.Pass)
32+
if (result == TagRule.Pass)
3333
validate(rules.tail, k, v)
3434
else
3535
result

atlas-core/src/main/scala/com/netflix/atlas/core/validation/HasKeyRule.scala

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ import com.typesafe.config.Config
2828
case class HasKeyRule(key: String) extends Rule {
2929

3030
def validate(tags: SmallHashMap[String, String]): ValidationResult = {
31-
if (tags.contains(key)) ValidationResult.Pass else failure(s"missing '$key': ${tags.keys}")
31+
if (tags.contains(key))
32+
ValidationResult.Pass
33+
else
34+
failure(s"missing '$key': ${tags.keys}", tags)
3235
}
3336
}
3437

atlas-core/src/main/scala/com/netflix/atlas/core/validation/KeyLengthRule.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ import com.typesafe.config.Config
2727
*/
2828
case class KeyLengthRule(minLength: Int, maxLength: Int) extends TagRule {
2929

30-
def validate(k: String, v: String): ValidationResult = {
30+
def validate(k: String, v: String): String = {
3131
k.length match {
3232
case len if len > maxLength =>
33-
failure(s"key too long: [$k] ($len > $maxLength)")
33+
s"key too long: [$k] ($len > $maxLength)"
3434
case len if len < minLength =>
35-
failure(s"key too short: [$k] ($len < $minLength)")
35+
s"key too short: [$k] ($len < $minLength)"
3636
case _ =>
37-
ValidationResult.Pass
37+
TagRule.Pass
3838
}
3939
}
4040
}

atlas-core/src/main/scala/com/netflix/atlas/core/validation/KeyPatternRule.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ import com.typesafe.config.Config
2828
*/
2929
case class KeyPatternRule(pattern: Pattern) extends TagRule {
3030

31-
def validate(k: String, v: String): ValidationResult = {
32-
if (pattern.matcher(k).matches()) ValidationResult.Pass
31+
def validate(k: String, v: String): String = {
32+
if (pattern.matcher(k).matches()) TagRule.Pass
3333
else {
34-
failure(s"key doesn't match pattern '$pattern': [$k]")
34+
s"key doesn't match pattern '$pattern': [$k]"
3535
}
3636
}
3737
}

atlas-core/src/main/scala/com/netflix/atlas/core/validation/MaxUserTagsRule.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ case class MaxUserTagsRule(limit: Int) extends Rule {
3737
}
3838
if (count <= limit) ValidationResult.Pass
3939
else {
40-
failure(s"too many user tags: $count > $limit")
40+
failure(s"too many user tags: $count > $limit", tags)
4141
}
4242
}
4343
}

atlas-core/src/main/scala/com/netflix/atlas/core/validation/NameValueLengthRule.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import com.typesafe.config.Config
3535
*/
3636
case class NameValueLengthRule(nameRule: TagRule, valueRule: TagRule) extends TagRule {
3737

38-
def validate(k: String, v: String): ValidationResult = {
38+
override def validate(k: String, v: String): String = {
3939
if (k == "name") nameRule.validate(k, v) else valueRule.validate(k, v)
4040
}
4141
}

atlas-core/src/main/scala/com/netflix/atlas/core/validation/ReservedKeyRule.scala

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ import com.typesafe.config.Config
3131
*/
3232
case class ReservedKeyRule(prefix: String, allowedKeys: Set[String]) extends TagRule {
3333

34-
override def validate(k: String, v: String): ValidationResult = {
34+
override def validate(k: String, v: String): String = {
3535
if (k.startsWith(prefix) && !allowedKeys.contains(k))
36-
failure(s"invalid key for reserved prefix '$prefix': $k")
36+
s"invalid key for reserved prefix '$prefix': $k"
3737
else
38-
ValidationResult.Pass
38+
TagRule.Pass
3939
}
4040
}
4141

atlas-core/src/main/scala/com/netflix/atlas/core/validation/Rule.scala

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ trait Rule {
4646
/**
4747
* Helper for generating the failure response.
4848
*/
49-
protected def failure(reason: String): ValidationResult = {
50-
ValidationResult.Fail(ruleName, reason)
49+
protected def failure(reason: String, tags: Map[String, String]): ValidationResult = {
50+
ValidationResult.Fail(ruleName, reason, tags)
5151
}
5252
}
5353

@@ -103,10 +103,12 @@ object Rule {
103103

104104
@scala.annotation.tailrec
105105
def validate(tags: Map[String, String], rules: List[Rule]): ValidationResult = {
106-
if (rules.isEmpty) ValidationResult.Pass
107-
else {
108-
val res = rules.head.validate(tags)
109-
if (res.isFailure) res else validate(tags, rules.tail)
106+
rules match {
107+
case r :: rs =>
108+
val result = r.validate(tags)
109+
if (result.isFailure) result else validate(tags, rs)
110+
case Nil =>
111+
ValidationResult.Pass
110112
}
111113
}
112114
}

atlas-core/src/main/scala/com/netflix/atlas/core/validation/TagRule.scala

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,23 @@ trait TagRule extends Rule {
2525
def validate(tags: SmallHashMap[String, String]): ValidationResult = {
2626
val iter = tags.entriesIterator
2727
while (iter.hasNext) {
28-
val res = validate(iter.key, iter.value)
29-
if (res.isFailure) return res
28+
val result = validate(iter.key, iter.value)
29+
if (result != TagRule.Pass) return failure(result, tags)
3030
iter.nextEntry()
3131
}
3232
ValidationResult.Pass
3333
}
3434

35-
def validate(k: String, v: String): ValidationResult
35+
/**
36+
* Check the key/value pair and return `null` if it is valid or a reason string if
37+
* there is a validation failure. The `null` type for success is used to avoid allocations
38+
* or other overhead since the validation checks tend to be a hot path.
39+
*/
40+
def validate(k: String, v: String): String
41+
}
42+
43+
object TagRule {
44+
45+
/** Null string to make the code easier to follow when using null for a passing result. */
46+
val Pass: String = null
3647
}

atlas-core/src/main/scala/com/netflix/atlas/core/validation/ValidCharactersRule.scala

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,17 @@ import com.typesafe.config.Config
4040
case class ValidCharactersRule(defaultSet: AsciiSet, overrides: Map[String, AsciiSet])
4141
extends TagRule {
4242

43-
def validate(k: String, v: String): ValidationResult = {
43+
def validate(k: String, v: String): String = {
4444
if (!defaultSet.containsAll(k)) {
45-
return failure(s"invalid characters in key: [$k] ([$defaultSet] are allowed)")
45+
return s"invalid characters in key: [$k] ([$defaultSet] are allowed)"
4646
}
4747

4848
val valueSet = overrides(k)
4949
if (!valueSet.containsAll(v)) {
50-
return failure(s"invalid characters in value: $k = [$v] ([$valueSet] are allowed)")
50+
return s"invalid characters in value: $k = [$v] ([$valueSet] are allowed)"
5151
}
5252

53-
ValidationResult.Pass
53+
TagRule.Pass
5454
}
5555
}
5656

0 commit comments

Comments
 (0)