Skip to content

Commit

Permalink
[mdatagen] Fix metric tests on boolean attributes (open-telemetry#11169)
Browse files Browse the repository at this point in the history
#### Description

Since commit 8f3ca8a, `mdatagen` improperly generates metric tests:
boolean attributes are always expected to be `true` in the output, no
matter what the injected test value was. For example, [this line in
collector-contrib](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/19b01db307d984e62f81f1880b63327ba092cd81/receiver/apachesparkreceiver/internal/metadata/generated_metrics_test.go#L1250)
is incorrectly changed to expect `true` instead of `false` after
updating mdatagen and running it again.

The reason is [this
line](https://github.com/open-telemetry/opentelemetry-collector/blob/8f3ca8aaf58539e5afab54d365f97624efe7cbc3/cmd/mdatagen/templates/metrics_test.go.tmpl#L182),
which attempts to generate `assert.True` or `assert.False` based on the
expected test value. However, `(attributeInfo $attr).TestValue` does not
return a boolean but the *string* `true` or `false`, both of which are
considered "truthy" by the templating engine.

#### Testing
I manually rebuilt mdatagen, ran `make generate` in collector-contrib,
and ran the tests, to confirm that the test related to the previously
mentioned change was no longer failing.
  • Loading branch information
jade-guiton-dd authored Sep 16, 2024
1 parent 232dff7 commit 74729e7
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 11 deletions.
25 changes: 25 additions & 0 deletions .chloggen/fix-mdatagen-metrics-bool-attr.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: mdatagen

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix incorrect generation of metric tests with boolean attributes.

# One or more tracking issues or pull requests related to the change
issues: [11169]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
1 change: 1 addition & 0 deletions cmd/mdatagen/internal/samplereceiver/documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ metrics:
| ---- | ----------- | ------ |
| string_attr | Attribute with any string value. | Any Str |
| boolean_attr | Attribute with a boolean value. | Any Bool |
| boolean_attr2 | Another attribute with a boolean value. | Any Bool |
### optional.metric.empty_unit
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 11 additions & 5 deletions cmd/mdatagen/internal/samplereceiver/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,27 +43,27 @@ resource_attributes:

map.resource.attr:
description: Resource attribute with a map value.
type: map
type: map
enabled: true

string.resource.attr_disable_warning:
description: Resource attribute with any string value.
type: string
enabled: true
enabled: true
warnings:
if_enabled_not_set: This resource_attribute will be disabled by default soon.

string.resource.attr_remove_warning:
description: Resource attribute with any string value.
type: string
enabled: false
enabled: false
warnings:
if_configured: This resource_attribute is deprecated and will be removed soon.

string.resource.attr_to_be_removed:
description: Resource attribute with any string value.
type: string
enabled: true
enabled: true
warnings:
if_enabled: This resource_attribute is deprecated and will be removed soon.

Expand All @@ -86,6 +86,12 @@ attributes:
description: Attribute with a boolean value.
type: bool

# This 2nd boolean attribute allows us to test both values for boolean attributes,
# as test values are based on the parity of the attribute name length.
boolean_attr2:
description: Another attribute with a boolean value.
type: bool

slice_attr:
description: Attribute with a slice value.
type: slice
Expand Down Expand Up @@ -114,7 +120,7 @@ metrics:
unit: "1"
gauge:
value_type: double
attributes: [string_attr, boolean_attr]
attributes: [string_attr, boolean_attr, boolean_attr2]
warnings:
if_configured: This metric is deprecated and will be removed soon.

Expand Down
9 changes: 8 additions & 1 deletion cmd/mdatagen/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,13 @@ func TestLoadMetadata(t *testing.T) {
},
FullName: "boolean_attr",
},
"boolean_attr2": {
Description: "Another attribute with a boolean value.",
Type: ValueType{
ValueType: pcommon.ValueTypeBool,
},
FullName: "boolean_attr2",
},
"slice_attr": {
Description: "Attribute with a slice value.",
Type: ValueType{
Expand Down Expand Up @@ -190,7 +197,7 @@ func TestLoadMetadata(t *testing.T) {
Gauge: &gauge{
MetricValueType: MetricValueType{pmetric.NumberDataPointValueTypeDouble},
},
Attributes: []attributeName{"string_attr", "boolean_attr"},
Attributes: []attributeName{"string_attr", "boolean_attr", "boolean_attr2"},
},
"optional.metric.empty_unit": {
Enabled: false,
Expand Down
2 changes: 1 addition & 1 deletion cmd/mdatagen/templates/metrics_test.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func TestMetricsBuilder(t *testing.T) {
attrVal, ok {{ if eq $i 0 }}:{{ end }}= dp.Attributes().Get("{{ (attributeInfo $attr).Name }}")
assert.True(t, ok)
{{- if eq (attributeInfo $attr).Type.String "Bool"}}
assert.{{- if (attributeInfo $attr).TestValue }}True{{ else }}False{{- end }}(t, attrVal.{{ (attributeInfo $attr).Type }}()
assert.{{- if eq (attributeInfo $attr).TestValue "true" }}True{{ else }}False{{- end }}(t, attrVal.{{ (attributeInfo $attr).Type }}()
{{- else }}
assert.EqualValues(t, {{ (attributeInfo $attr).TestValue }}, attrVal.{{ (attributeInfo $attr).Type }}()
{{- end }}
Expand Down

0 comments on commit 74729e7

Please sign in to comment.