Skip to content

Conversation

trentm
Copy link
Contributor

@trentm trentm commented May 15, 2025

For example the 'db.system.name' enum is stable, but its values are
of mixed stability: e.g. "mysql" and "postgresql" are stable, but
"cassandra" and "sqlite" are stability: development. The template
generation did not account for this and some enum value exports
made it into the "stable" exports in some releases.

The following unstable exports incorrectly are included in @opentelemetry/semantic-conventions version 1.33.0. This PR moves them back to the unstable "incubating" entry-point.

DB_SYSTEM_NAME_VALUE_ACTIAN_INGRES = "actian.ingres"
DB_SYSTEM_NAME_VALUE_AWS_DYNAMODB = "aws.dynamodb"
DB_SYSTEM_NAME_VALUE_AWS_REDSHIFT = "aws.redshift"
DB_SYSTEM_NAME_VALUE_AZURE_COSMOSDB = "azure.cosmosdb"
DB_SYSTEM_NAME_VALUE_CASSANDRA = "cassandra"
DB_SYSTEM_NAME_VALUE_CLICKHOUSE = "clickhouse"
DB_SYSTEM_NAME_VALUE_COCKROACHDB = "cockroachdb"
DB_SYSTEM_NAME_VALUE_COUCHBASE = "couchbase"
DB_SYSTEM_NAME_VALUE_COUCHDB = "couchdb"
DB_SYSTEM_NAME_VALUE_DERBY = "derby"
DB_SYSTEM_NAME_VALUE_ELASTICSEARCH = "elasticsearch"
DB_SYSTEM_NAME_VALUE_FIREBIRDSQL = "firebirdsql"
DB_SYSTEM_NAME_VALUE_GCP_SPANNER = "gcp.spanner"
DB_SYSTEM_NAME_VALUE_GEODE = "geode"
DB_SYSTEM_NAME_VALUE_H2DATABASE = "h2database"
DB_SYSTEM_NAME_VALUE_HBASE = "hbase"
DB_SYSTEM_NAME_VALUE_HIVE = "hive"
DB_SYSTEM_NAME_VALUE_HSQLDB = "hsqldb"
DB_SYSTEM_NAME_VALUE_IBM_DB2 = "ibm.db2"
DB_SYSTEM_NAME_VALUE_IBM_INFORMIX = "ibm.informix"
DB_SYSTEM_NAME_VALUE_IBM_NETEZZA = "ibm.netezza"
DB_SYSTEM_NAME_VALUE_INFLUXDB = "influxdb"
DB_SYSTEM_NAME_VALUE_INSTANTDB = "instantdb"
DB_SYSTEM_NAME_VALUE_INTERSYSTEMS_CACHE = "intersystems.cache"
DB_SYSTEM_NAME_VALUE_MEMCACHED = "memcached"
DB_SYSTEM_NAME_VALUE_MONGODB = "mongodb"
DB_SYSTEM_NAME_VALUE_NEO4J = "neo4j"
DB_SYSTEM_NAME_VALUE_OPENSEARCH = "opensearch"
DB_SYSTEM_NAME_VALUE_ORACLE_DB = "oracle.db"
DB_SYSTEM_NAME_VALUE_OTHER_SQL = "other_sql"
DB_SYSTEM_NAME_VALUE_REDIS = "redis"
DB_SYSTEM_NAME_VALUE_SAP_HANA = "sap.hana"
DB_SYSTEM_NAME_VALUE_SAP_MAXDB = "sap.maxdb"
DB_SYSTEM_NAME_VALUE_SOFTWAREAG_ADABAS = "softwareag.adabas"
DB_SYSTEM_NAME_VALUE_SQLITE = "sqlite"
DB_SYSTEM_NAME_VALUE_TERADATA = "teradata"
DB_SYSTEM_NAME_VALUE_TRINO = "trino"

Note

There was one other case of this. The following unstable export incorrectly is included in @opentelemetry/semantic-conventions versions >=1.26.0 <=1.33.0:

NETWORK_TRANSPORT_VALUE_QUIC = "quic"

However, this change is not removing this export from the stable entry point, because it has been included for so long. In open-telemetry/semantic-conventions#2275 it was marked as stable, and will be in the semconv v1.34.0 release.

Open questions

  • What to do about the relatively long-standing NETWORK_TRANSPORT_VALUE_QUIC? The same Q for the Java semconv package was asked here: https://github.com/open-telemetry/semantic-conventions-java/pull/220/files#r2089934389
    • Answer: grandfather it in. The special casing in "attributes.ts.j2" can be removed for v1.34.0.
  • What to do about the DB_SYSTEM_NAME_VALUE_* exports. These have only been out for about a week ('1.33.0': '2025-05-07T18:38:31.191Z') so the answer here might differ.
    • Answer: Do a v1.33.1 release that moves these back to the unstable entry point. This is "Option 4" as described below.

trentm added 4 commits May 15, 2025 09:40
…' arg to semconv_attributes

This is preferred to exclude_stability, b/c it is expected to be more
resilient with other stability names used for unstable, e.g. 'development',
'rc', 'beta', etc.

This results in the exact same generated files.
… on enum values

There is no functional change to the generated semconv .ts files.
weaver v0.9.0 provided the 'file_name' template config, which is a
little cleaner than the in-template .set_file_name usage
…ere accidentally included in the stable export

For example the 'db.system.name' enum is stable, but its values are
of mixed stability: e.g. "mysql" and "postgresql" are stable, but
"cassandra" and "sqlite" are `stability: development`. The template
generation did not account for this and some enum value exports
made it into the "stable" exports in some releases.

The following *un*stable export incorrectly is included in `@opentelemetry/semantic-conventions` versions `>=1.26.0 <=1.33.0`:

```
NETWORK_TRANSPORT_VALUE_QUIC = "quic"
```

The following *un*stable exports incorrectly are included in `@opentelemetry/semantic-conventions` version `1.33.0`:

```
DB_SYSTEM_NAME_VALUE_ACTIAN_INGRES = "actian.ingres"
DB_SYSTEM_NAME_VALUE_AWS_DYNAMODB = "aws.dynamodb"
DB_SYSTEM_NAME_VALUE_AWS_REDSHIFT = "aws.redshift"
DB_SYSTEM_NAME_VALUE_AZURE_COSMOSDB = "azure.cosmosdb"
DB_SYSTEM_NAME_VALUE_CASSANDRA = "cassandra"
DB_SYSTEM_NAME_VALUE_CLICKHOUSE = "clickhouse"
DB_SYSTEM_NAME_VALUE_COCKROACHDB = "cockroachdb"
DB_SYSTEM_NAME_VALUE_COUCHBASE = "couchbase"
DB_SYSTEM_NAME_VALUE_COUCHDB = "couchdb"
DB_SYSTEM_NAME_VALUE_DERBY = "derby"
DB_SYSTEM_NAME_VALUE_ELASTICSEARCH = "elasticsearch"
DB_SYSTEM_NAME_VALUE_FIREBIRDSQL = "firebirdsql"
DB_SYSTEM_NAME_VALUE_GCP_SPANNER = "gcp.spanner"
DB_SYSTEM_NAME_VALUE_GEODE = "geode"
DB_SYSTEM_NAME_VALUE_H2DATABASE = "h2database"
DB_SYSTEM_NAME_VALUE_HBASE = "hbase"
DB_SYSTEM_NAME_VALUE_HIVE = "hive"
DB_SYSTEM_NAME_VALUE_HSQLDB = "hsqldb"
DB_SYSTEM_NAME_VALUE_IBM_DB2 = "ibm.db2"
DB_SYSTEM_NAME_VALUE_IBM_INFORMIX = "ibm.informix"
DB_SYSTEM_NAME_VALUE_IBM_NETEZZA = "ibm.netezza"
DB_SYSTEM_NAME_VALUE_INFLUXDB = "influxdb"
DB_SYSTEM_NAME_VALUE_INSTANTDB = "instantdb"
DB_SYSTEM_NAME_VALUE_INTERSYSTEMS_CACHE = "intersystems.cache"
DB_SYSTEM_NAME_VALUE_MEMCACHED = "memcached"
DB_SYSTEM_NAME_VALUE_MONGODB = "mongodb"
DB_SYSTEM_NAME_VALUE_NEO4J = "neo4j"
DB_SYSTEM_NAME_VALUE_OPENSEARCH = "opensearch"
DB_SYSTEM_NAME_VALUE_ORACLE_DB = "oracle.db"
DB_SYSTEM_NAME_VALUE_OTHER_SQL = "other_sql"
DB_SYSTEM_NAME_VALUE_REDIS = "redis"
DB_SYSTEM_NAME_VALUE_SAP_HANA = "sap.hana"
DB_SYSTEM_NAME_VALUE_SAP_MAXDB = "sap.maxdb"
DB_SYSTEM_NAME_VALUE_SOFTWAREAG_ADABAS = "softwareag.adabas"
DB_SYSTEM_NAME_VALUE_SQLITE = "sqlite"
DB_SYSTEM_NAME_VALUE_TERADATA = "teradata"
DB_SYSTEM_NAME_VALUE_TRINO = "trino"
```
@trentm trentm self-assigned this May 15, 2025
Copy link

codecov bot commented May 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.03%. Comparing base (4e2703c) to head (36ffadd).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5690   +/-   ##
=======================================
  Coverage   95.03%   95.03%           
=======================================
  Files         310      310           
  Lines        7992     7992           
  Branches     1614     1614           
=======================================
  Hits         7595     7595           
  Misses        397      397           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trentm
Copy link
Contributor Author

trentm commented May 15, 2025

Implementation note: While the weaver config and templates were split into two dirs, it may be useful to see how scripts/semconv/templates/registry/ts-stable and scripts/semconv/templates/registry/ts-experimental differ:

small diffs in weaver config and templates
% diff -ur ts-stable ts-experimental
diff -ur ts-stable/attributes.ts.j2 ts-experimental/attributes.ts.j2
--- ts-stable/attributes.ts.j2	2025-05-15 14:55:51
+++ ts-experimental/attributes.ts.j2	2025-05-15 14:55:51
@@ -21,6 +21,7 @@

 {% for attribute in ctx.attributes | attribute_sort %}
 {% if attribute.name not in params.excluded_attributes %}
+{% if attribute is not stable %}
 {{d.docstring(attribute, "attribute")}}
 {% if attribute.type is not template_type %}
 export const ATTR_{{ attribute.name | screaming_snake_case }} = '{{attribute.name}}' as const;
@@ -29,9 +30,10 @@
 export const ATTR_{{ attribute.name | screaming_snake_case }} = (key: string) => `{{attribute.name}}.${key}`;

 {% endif %}
+{% endif %}
 {% if attribute.type is mapping %}
 {% for espec in attribute.type.members | sort(attribute='value') %}
-{% if espec is stable %}
+{% if espec is not stable %}
 /**
  * Enum value {{ espec.value | print_member_value }} for attribute {@link ATTR_{{ attribute.name | screaming_snake_case }}}.
  */
diff -ur ts-stable/weaver.yaml ts-experimental/weaver.yaml
--- ts-stable/weaver.yaml	2025-05-15 15:04:39
+++ ts-experimental/weaver.yaml	2025-05-15 15:06:34
@@ -1,20 +1,28 @@
-# ts-stable/... generates the "semantic-conventions/src/stable_*.ts" files.
+# ts-experimental/... generates the "semantic-conventions/src/experimental_*.ts"
+# files.

+# Notes:
+# - Use `""` and `null` with `exclude_stability` to skip attributes/metrics that
+#   accidentally do not have a stability set
+#   (e.g. https://github.com/open-telemetry/semantic-conventions/issues/1777).
 templates:
   - pattern: attributes.ts.j2
-    file_name: "stable_attributes.ts"
+    file_name: "experimental_attributes.ts"
+    # This "exclude_stability" does *not* exclude "stable" because it needs
+    # to process *stable* enums that might have *unstable* members. (e.g.,
+    # `db.system.name` is stable, but some of its enum values are not.)
     filter: >
       semconv_attributes({
-        "stable_only": true
+        "exclude_stability": ["", null]
       }) | {
         attributes: .
       }
     application_mode: single
   - pattern: metrics.ts.j2
-    file_name: "stable_metrics.ts"
+    file_name: "experimental_metrics.ts"
     filter: >
       semconv_metrics({
-        "stable_only": true
+        "exclude_stability": ["stable", "", null]
       }) | {
         metrics: .
       }

@trentm
Copy link
Contributor Author

trentm commented May 15, 2025

What to do about removing exports from the "stable" entry-point? Options:

  1. Just remove them. Release as v1.33.1 and hope it doesn't break anyone. (Honestly I doubt that it will.)
  2. Keep the exports in the stable entry-point, but add @experimental and @deprecated jsdoc tags to their comment blocks explaining the issue. This is problematic because currently the .../incubating entry-point exports both the stable and experimental exports... and having, e.g., DB_SYSTEM_NAME_VALUE_SQLITE defined in both stable_attributes.ts and experimental_attributes.ts makes handling that a pain. The only way I can think to handle that would be to have this line in "index-incubating.ts" be expanded to explicitly list all exports except those few accidental ones. There are currently about 130 of them, so not too bad.
  3. Do option 2, but just for NETWORK_TRANSPORT_VALUE_QUIC because it has been there for a number of releases.

Update: semconv is going to grandfather in ..._QUIC (open-telemetry/semantic-conventions#2275). Presumably that'll be in a 1.33.1 or 1.34.0 release. So another option:

  1. If that ^^ release is v1.34.0, let's do a v1.33.1 release that drops the unstable DB_SYSTEM_NAME_VALUE_* exports (like in Option 1), but keeps the NETWORK_TRANSPORT_VALUE_QUIC. Then we do a v1.34.0 release which will be able to remove the special handling for NETWORK_TRANSPORT_VALUE_QUIC.

@trentm
Copy link
Contributor Author

trentm commented May 15, 2025

  1. Just remove them. Release as v1.33.1 and hope it doesn't break anyone. (Honestly I doubt that it will.)

[narrator] Look at this: https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2671/files/53585cb522a3f7b419391fd761fda071b21be2be..88fb85c6b9e5a0fc9ed1260fc763d2c34846c5b3#diff-ce92a05c80ac8c55280922ac396fdea2f64de6902642f6ec4903bb32cb8087ccR20-R21

This (not yet merged) PR was going to DB_SYSTEM_NAME_VALUE_SQLITE from @opentelemetry/[email protected].

Not a breakage, because not merged, but we should decide on this soonish.

@maryliag
Copy link
Contributor

maryliag commented May 16, 2025

For what is worth, regarding db.system.name, the list you provide are not stable, not because of the names itself, but because we didn't have any prototypes or testing for all those cases. We wanted to make sure the metrics/spans/attributes can all be collected for all those databases before marking them stable, so we left to the vendor to do the testing, etc.
We don't foresee any changes coming on the database names itself, so might not be such a big deal having them stable/development for this case.

@trentm
Copy link
Contributor Author

trentm commented May 16, 2025

4. If that ^^ release is v1.34.0,

It will be a v1.34.0 release (planned for next week).

@trentm
Copy link
Contributor Author

trentm commented May 16, 2025

^^ commit cfe1411 makes the changes for Option 4.

@trentm trentm changed the title fix(semantic-conventions): unstable enum *values* from stable enums were accidentally included in the stable export fix(semantic-conventions): Remove the subset of DB_SYSTEM_NAME_VALUE_* exports that are unstable from the @opentelemetry/semantic-conventions entry point. May 16, 2025
@trentm trentm marked this pull request as ready for review May 16, 2025 21:45
@trentm trentm requested a review from a team as a code owner May 16, 2025 21:45
@trentm
Copy link
Contributor Author

trentm commented May 16, 2025

@maryliag Thanks. The template handling to avoid this is generic, so a future v1.34.0 release would move them back to the unstable entry-point. I think it is a little clearer to the user if the state in v1.33.0 is considered a bug, and it is fixed in a v1.33.1 point release.

registry generate \
--registry=/source \
--templates=/weaver/templates \
ts-experimental \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review note: This PR splits the templates for stable and experimental to two separate dirs, so that the logic can differ between them. Comment #5690 (comment) includes a (slightly out of date) diff of the two dirs to show how similar they are.

I tried to share the unchanged files ("metrics.ts.j2", "docstring.ts.j2"), but weaver borks when you try to use files outside of the root dir.

@pichlermarc
Copy link
Member

  1. If that ^^ release is v1.34.0, let's do a v1.33.1 release that drops the unstable DB_SYSTEM_NAME_VALUE_* exports (like in Option 1), but keeps the NETWORK_TRANSPORT_VALUE_QUIC. Then we do a v1.34.0 release which will be able to remove the special handling for NETWORK_TRANSPORT_VALUE_QUIC.

I think that's the way. 👍

@trentm trentm added this pull request to the merge queue May 20, 2025
Merged via the queue into open-telemetry:main with commit 1c8ec3b May 20, 2025
19 checks passed
@trentm trentm deleted the trentm-semconv-unstable-enum-values-take2 branch May 20, 2025 16:09
trentm added a commit to trentm/opentelemetry-js that referenced this pull request May 20, 2025
This removes the special-casing of NETWORK_TRANSPORT_VALUE_QUIC
which is now marked stable in semconv.

Refs: open-telemetry#5690
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants