Skip to content

Commit

Permalink
Validate that enum member IDs are valid (#64)
Browse files Browse the repository at this point in the history
  • Loading branch information
Oberon00 authored Sep 15, 2021
1 parent 5175c77 commit 29fabe9
Show file tree
Hide file tree
Showing 55 changed files with 235 additions and 1,013 deletions.
8 changes: 8 additions & 0 deletions semantic-conventions/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@

Please update the changelog as part of any significant pull request.

## v0.6.0

- Enforce enum member IDs follow the same rules as other IDs
([#64](https:/open-telemetry/build-tools/pull/64)).
- Improve some enum-related error messages to point to more precise
locations
(also [#64](https:/open-telemetry/build-tools/pull/64)).

## v0.5.0

- Add event semantic convention type & events span field
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ def parse_id(attribute):
try:
attr_type = EnumAttributeType.parse(attr_val)
except ValidationError as e:
if e.line != 0: # Does the error already have a position?
raise
position = attribute.lc.data["type"]
raise ValidationError.from_yaml_pos(position, e.message) from e
brief = attribute["brief"]
Expand Down Expand Up @@ -432,32 +434,39 @@ def parse(attribute_type):
else False
)
members = []
if attribute_type["members"] is None or len(attribute_type["members"]) < 1:
# Missing members - rise the exception and fill the missing data in the parent
raise ValidationError(0, 0, "Enumeration without values!")
if (
not isinstance(attribute_type["members"], CommentedSeq)
or len(attribute_type["members"]) < 1
):
raise ValidationError.from_yaml_pos(
attribute_type.lc.data["members"], "Enumeration without members!"
)

allowed_keys = ["id", "value", "brief", "note"]
mandatory_keys = ["id", "value"]
for member in attribute_type["members"]:
validate_values(member, allowed_keys, mandatory_keys)
if not EnumAttributeType.is_valid_enum_value(member["value"]):
raise ValidationError(
0, 0, "Invalid value used in enum: <{}>".format(member["value"])
raise ValidationError.from_yaml_pos(
member.lc.data["value"][:2],
"Invalid value used in enum: <{}>".format(member["value"]),
)
validate_id(member["id"], member.lc.data["id"])
members.append(
EnumMember(
member_id=member["id"],
value=member["value"],
brief=member.get("brief").strip()
if "brief" in member
else member["id"],
note=member.get("note").strip() if "note" in member else "",
brief=member.get("brief", member["id"]).strip(),
note=member.get("note", "").strip(),
)
)
enum_type = AttributeType.get_type(members[0].value)
for m in members:
for myaml, m in zip(attribute_type["members"], members):
if enum_type != AttributeType.get_type(m.value):
raise ValidationError(0, 0, "Enumeration type inconsistent!")
raise ValidationError.from_yaml_pos(
myaml.lc.data["value"],
"Enumeration member does not have type {}!".format(enum_type),
)
return EnumAttributeType(custom_values, members, enum_type)


Expand Down
10 changes: 6 additions & 4 deletions semantic-conventions/src/opentelemetry/semconv/model/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@

from opentelemetry.semconv.model.exceptions import ValidationError


ID_RE = re.compile("([a-z](\\.?[a-z0-9_-]+)+)")
"""Identifiers must start with a lowercase ASCII letter and
contain only lowercase, digits 0-9, underscore, dash (not recommended) and dots.
Each dot must be followed by at least one allowed non-dot character."""


def validate_id(semconv_id, position):
if not ID_RE.fullmatch(semconv_id):
raise ValidationError.from_yaml_pos(
position,
"Invalid id {}. Semantic Convention ids MUST be {}".format(
"Invalid id {}. Semantic Convention ids MUST match {}".format(
semconv_id, ID_RE.pattern
),
)
Expand All @@ -45,7 +47,7 @@ def validate_values(yaml, keys, mandatory=()):
def check_no_missing_keys(yaml, mandatory):
missing = list(set(mandatory) - set(yaml))
if missing:
position = yaml.lc.data[list(yaml)[0]]
position = (yaml.lc.line, yaml.lc.col)
msg = "Missing keys: {}".format(missing)
raise ValidationError.from_yaml_pos(position, msg)

Expand All @@ -59,7 +61,7 @@ def __init__(self, yaml_node):
self.id = yaml_node.get("id").strip()
self.brief = str(yaml_node.get("brief")).strip()

self._position = [yaml_node.lc.line, yaml_node.lc.col]
self._position = (yaml_node.lc.line, yaml_node.lc.col)

@classmethod
def validate_keys(cls, node):
Expand Down
2 changes: 1 addition & 1 deletion semantic-conventions/src/opentelemetry/semconv/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@
# See the License for the specific language governing permissions and
# limitations under the License.

__version__ = "0.5.0"
__version__ = "0.6.0"
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ groups:
type:
allow_custom_values: false
members:
- id: IP.TCP
- id: ip.tcp
value: "IP.TCP"
- id: IP.UDP
- id: ip.udp
value: "IP.UDP"
- id: IP
- id: ip
value: "IP"
brief: 'Another IP-based protocol'
- id: Unix
- id: unix
value: "Unix"
brief: 'Unix Domain socket. See below.'
- id: pipe
Expand Down Expand Up @@ -83,4 +83,4 @@ groups:
Scopes or granted authorities the client currently possesses extracted from token
or application security context. The value would come from the scope associated
with an OAuth 2.0 Access Token or an attribute value in a SAML 2.0 Assertion.
examples: 'read:message, write:files'
examples: 'read:message, write:files'
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,19 @@ groups:
# encode checks that only accept the listed values.
allow_custom_values: true
members:
- id: HTTP_1_0
- id: http_1_0
value: '1.0'
brief: 'HTTP 1.0'
- id: HTTP_1_1
- id: http_1_1
value: '1.1'
brief: 'HTTP 1.1'
- id: HTTP_2_0
- id: http_2_0
value: '2.0'
brief: 'HTTP 2'
- id: SPDY
- id: spdy
value: 'SPDY'
brief: 'SPDY protocol.'
- id: QUIC
- id: quic
value: 'QUIC'
brief: 'QUIC protocol.'
brief: 'Kind of HTTP protocol used'
Expand Down Expand Up @@ -113,4 +113,4 @@ groups:
- [http.url]
- [http.scheme, http.host, http.target]
- [http.scheme, http.server_name, net.host.port, http.target]
- [http.scheme, net.host.name, net.host.port, http.target]
- [http.scheme, net.host.name, net.host.port, http.target]
10 changes: 5 additions & 5 deletions semantic-conventions/src/tests/data/markdown/empty/general.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ groups:
type:
allow_custom_values: false
members:
- id: IP.TCP
- id: ip.tcp
value: "IP.TCP"
- id: IP.UDP
- id: ip.udp
value: "IP.UDP"
- id: IP
- id: ip
value: "IP"
brief: 'Another IP-based protocol'
- id: Unix
- id: unix
value: "Unix"
brief: 'Unix Domain socket. See below.'
- id: pipe
Expand Down Expand Up @@ -83,4 +83,4 @@ groups:
Scopes or granted authorities the client currently possesses extracted from token
or application security context. The value would come from the scope associated
with an OAuth 2.0 Access Token or an attribute value in a SAML 2.0 Assertion.
examples: 'read:message, write:files'
examples: 'read:message, write:files'
28 changes: 1 addition & 27 deletions semantic-conventions/src/tests/data/markdown/empty/http.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,32 +43,6 @@ groups:
type: string
brief: '[HTTP reason phrase](https://tools.ietf.org/html/rfc7230#section-3.1.2).'
examples: ['OK']
- id: flavor
type:
# Default value: `true`. If false, it helps the code gen tool to
# encode checks that only accept the listed values.
allow_custom_values: true
members:
- id: HTTP_1_0
value: '1.0'
brief: 'HTTP 1.0'
- id: HTTP_1_1
value: '1.1'
brief: 'HTTP 1.1'
- id: HTTP_2_0
value: '2.0'
brief: 'HTTP 2'
- id: SPDY
value: 'SPDY'
brief: 'SPDY protocol.'
- id: QUIC
value: 'QUIC'
brief: 'QUIC protocol.'
brief: 'Kind of HTTP protocol used'
note: >
If `net.transport` is not specified, it can be assumed to be `IP.TCP` except if `http.flavor`
is `QUIC`, in which case `IP.UDP` is assumed.
examples: ['1.0']
- id: user_agent
type: string
brief: 'Value of the [HTTP User-Agent](https://tools.ietf.org/html/rfc7231#section-5.5.3) header sent by the client.'
Expand Down Expand Up @@ -111,4 +85,4 @@ groups:
- [http.url]
- [http.scheme, http.host, http.target]
- [http.scheme, http.server_name, net.host.port, http.target]
- [http.scheme, net.host.name, net.host.port, http.target]
- [http.scheme, net.host.name, net.host.port, http.target]
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ groups:
type:
allow_custom_values: false
members:
- id: IP.TCP
- id: ip.tcp
value: "IP.TCP"
- id: IP.UDP
- id: ip.udp
value: "IP.UDP"
- id: IP
- id: ip
value: "IP"
brief: 'Another IP-based protocol'
- id: Unix
- id: unix
value: "Unix"
brief: 'Unix Domain socket. See below.'
- id: pipe
Expand Down Expand Up @@ -83,4 +83,4 @@ groups:
Scopes or granted authorities the client currently possesses extracted from token
or application security context. The value would come from the scope associated
with an OAuth 2.0 Access Token or an attribute value in a SAML 2.0 Assertion.
examples: 'read:message, write:files'
examples: 'read:message, write:files'
Original file line number Diff line number Diff line change
Expand Up @@ -43,32 +43,6 @@ groups:
type: string
brief: '[HTTP reason phrase](https://tools.ietf.org/html/rfc7230#section-3.1.2).'
examples: ['OK']
- id: flavor
type:
# Default value: `true`. If false, it helps the code gen tool to
# encode checks that only accept the listed values.
allow_custom_values: true
members:
- id: HTTP_1_0
value: '1.0'
brief: 'HTTP 1.0'
- id: HTTP_1_1
value: '1.1'
brief: 'HTTP 1.1'
- id: HTTP_2_0
value: '2.0'
brief: 'HTTP 2'
- id: SPDY
value: 'SPDY'
brief: 'SPDY protocol.'
- id: QUIC
value: 'QUIC'
brief: 'QUIC protocol.'
brief: 'Kind of HTTP protocol used'
note: >
If `net.transport` is not specified, it can be assumed to be `IP.TCP` except if `http.flavor`
is `QUIC`, in which case `IP.UDP` is assumed.
examples: ['1.0']
- id: user_agent
type: string
brief: 'Value of the [HTTP User-Agent](https://tools.ietf.org/html/rfc7231#section-5.5.3) header sent by the client.'
Expand Down Expand Up @@ -125,4 +99,4 @@ groups:
- [http.url]
- [http.scheme, http.host, http.target]
- [http.scheme, http.server_name, net.host.port, http.target]
- [http.scheme, net.host.name, net.host.port, http.target]
- [http.scheme, net.host.name, net.host.port, http.target]
34 changes: 17 additions & 17 deletions semantic-conventions/src/tests/data/markdown/enum_int/expected.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@

| Value | Description |
|---|---|
| `0` | OK |
| `1` | CANCELLED |
| `2` | UNKNOWN |
| `3` | INVALID_ARGUMENT |
| `4` | DEADLINE_EXCEEDED |
| `5` | NOT_FOUND |
| `6` | ALREADY_EXISTS |
| `7` | PERMISSION_DENIED |
| `8` | RESOURCE_EXHAUSTED |
| `9` | FAILED_PRECONDITION |
| `10` | ABORTED |
| `11` | OUT_OF_RANGE |
| `12` | UNIMPLEMENTED |
| `13` | INTERNAL |
| `14` | UNAVAILABLE |
| `15` | DATA_LOSS |
| `16` | UNAUTHENTICATED |
| `0` | ok |
| `1` | cancelled |
| `2` | unknown |
| `3` | invalid_argument |
| `4` | deadline_exceeded |
| `5` | not_found |
| `6` | already_exists |
| `7` | permission_denied |
| `8` | resource_exhausted |
| `9` | failed_precondition |
| `10` | aborted |
| `11` | out_of_range |
| `12` | unimplemented |
| `13` | internal |
| `14` | unavailable |
| `15` | data_loss |
| `16` | unauthenticated |
<!-- endsemconv -->
Loading

0 comments on commit 29fabe9

Please sign in to comment.