feat: add semconv generator for `semantic-conventions`-package #2083

weyert · 2021-04-08T01:26:09Z

Which problem is this PR solving?

Adds a build script which generate the files for the semantic-conventions-package
so it always in sync with the definitions in the specifications-repo

A few unspecified attributes are added manually to the semconv template to
allow existing code to pass, these are:

  // HTTP
  HTTP_ERROR_NAME: 'http.error_name',
  HTTP_ERROR_MESSAGE: 'http.error_message',
  HTTP_STATUS_TEXT: 'http.status_text',

  // GRPC
  GRPC_KIND: 'grpc.kind', // SERVER or CLIENT
  GRPC_METHOD: 'grpc.method',
  GRPC_STATUS_CODE: 'grpc.status_code',
  GRPC_ERROR_NAME: 'grpc.error_name',
  GRPC_ERROR_MESSAGE: 'grpc.error_message',

Fixes #2064

Short description of the changes

This change introduces a script generate.sh in the directory scripts/semconv which allows you to generate the constants which are defined in the semantic conventions section of the Opentelemetry specification.

The following to files are generated:

SemanticAttribute
ResourceAttribute

All the packages that depend on the @opentelemetry/semantic-conventions-package has been updated to accommodate this change.

To allow this version of the spec files to work with the semconv the
version of this tool had to be lowered to 0.2.1 as there is a breaking
change in 0.3.0 which removes support for the number-type and
because the latest published version of the otel spec is v1.1.0 which
still uses this number-type.

Adds a build script which generate the files for the `semenatic-conventions`-package so it always in sync with the definitions in the `specifications`-repo A few unspecified attributes are added manually to the semconv template to allow existing code to pass, these are: // HTTP HTTP_ERROR_NAME: 'http.error_name', HTTP_ERROR_MESSAGE: 'http.error_message', HTTP_STATUS_TEXT: 'http.status_text', // GRPC GRPC_KIND: 'grpc.kind', // SERVER or CLIENT GRPC_METHOD: 'grpc.method', GRPC_STATUS_CODE: 'grpc.status_code', GRPC_ERROR_NAME: 'grpc.error_name', GRPC_ERROR_MESSAGE: 'grpc.error_message',

… build script

Updates the different packages to fix the breaking change were there is only `SemanticAttribute` and `ResourceAttribute`-class defined in the `@opentelemetry/semantic-conventations`-package

…ules

codecov · 2021-04-08T01:32:40Z

Codecov Report

Merging #2083 (a5dc4fc) into main (7f7afa7) will increase coverage by 0.05%.
The diff coverage is 98.48%.

❗ Current head a5dc4fc differs from pull request most recent head 628bb71. Consider uploading reports for the commit 628bb71 to get more accurate results

@@            Coverage Diff             @@
##             main    #2083      +/-   ##
==========================================
+ Coverage   92.53%   92.59%   +0.05%     
==========================================
  Files         133      137       +4     
  Lines        4851     4872      +21     
  Branches     1002     1005       +3     
==========================================
+ Hits         4489     4511      +22     
+ Misses        362      361       -1

Impacted Files	Coverage Δ
packages/opentelemetry-tracing/src/enums.ts	`100.00% <ø> (ø)`
...es/opentelemetry-instrumentation-http/src/utils.ts	`99.04% <94.44%> (+<0.01%)`	⬆️
...-instrumentation-fetch/src/enums/AttributeNames.ts	`100.00% <100.00%> (ø)`
...s/opentelemetry-instrumentation-fetch/src/fetch.ts	`99.27% <100.00%> (ø)`
...es/opentelemetry-instrumentation-grpc/src/enums.ts	`100.00% <100.00%> (ø)`
...ry-instrumentation-grpc/src/grpc-js/clientUtils.ts	`92.59% <100.00%> (+0.09%)`	⬆️
...elemetry-instrumentation-grpc/src/grpc-js/index.ts	`90.81% <100.00%> (ø)`
...ry-instrumentation-grpc/src/grpc-js/serverUtils.ts	`91.66% <100.00%> (+0.11%)`	⬆️
...metry-instrumentation-grpc/src/grpc/clientUtils.ts	`88.40% <100.00%> (+0.17%)`	⬆️
...entelemetry-instrumentation-grpc/src/grpc/index.ts	`93.04% <100.00%> (ø)`
... and 11 more

Changed `generate.sh` to ensure it generates the files against the latest published version of the otel specification which is v1.1.0. To allow this version of the spec files to work with the `semconv` the version of this tool had to be lowered to 0.2.1 as there is a breaking change in 0.3.0 which removes support for the `number`-type.

vmarchaud · 2021-04-08T09:01:40Z

scripts/semconv/templates/SemanticAttributes.ts.j2

+
+  {%- if class == "SemanticAttribute" %}
+
+  // Manually defined and not YET in the YAML


is there plan to add them upstream ?

Good question, I am not sure. Might be worth considering. I have added them to avoid needing to change too much code

Most of these sound like obsolete attributes that are represented/named differently

grpc.kind should be represented as the Span Kind

grpc.status_code should be rpc.grpc.status_code

grpc.error_name is redundant with the code

gprc.status_message is probably worth filing a spec PR for (as rpc.grpc.status_message). The span status message might not be ideal to use for it, for the same reason we don't mark spans as failed in RecordException.

http.status_text was deemed useless and removed without replacement, see Remove semantic convention for http.status_text opentelemetry-specification#950.

http.error_name, http.status_text: No idea what these are.

I think they shouldn't be defined here anyway if they aren't in the semantic conventions. Anything in this package should be only things approved by spec and by including them we are giving them our implicit approval for third parties. Most people will assume anything they find in autocomplete here is "official"

I have moved them into the packages and updated the code accordingly
Also made sure that we use rpc.grpc.status_code instead of the previous one

weyert · 2021-04-08T10:20:12Z

I have to make some more changes related to this ticket: open-telemetry/build-tools#41

But looking forward hearing if this approach is fine. Having a second look I am not sure if we want to name it ResourceAttributes and SemanticAttributes (plural)

dyladan · 2021-04-08T12:23:19Z

I have to make some more changes related to this ticket: open-telemetry/build-tools#41

But looking forward hearing if this approach is fine. Having a second look I am not sure if we want to name it ResourceAttributes and SemanticAttributes (plural)

I would for sure prefer ResourceAttributes and SpanAttributes. Technically they are all semantic attributes so that name doesn't really add much.

- Removed all the semantic convention constants from the `@opentelemetry/semantic-conventions`-package that are not part of the Opentelemetry Specification v1.1.0 - Renamed the `ResourceAttribute`-class to `ResourceAttributes` - Renamed the `SemanticAttribute`-class to `SemanticAttributes` as `SpanAttributes` name conflicts with the interface defined in the `api`-package - Updated references in the packages to use the new names

weyert · 2021-04-08T17:59:02Z

I have to make some more changes related to this ticket: open-telemetry/build-tools#41
But looking forward hearing if this approach is fine. Having a second look I am not sure if we want to name it ResourceAttributes and SemanticAttributes (plural)

I would for sure prefer ResourceAttributes and SpanAttributes. Technically they are all semantic attributes so that name doesn't really add much.

I have renamed ResourceAttributes but for now using SemanticAttributes mainly because we already have SpanAttributes in the api-package which I think could cause confusion. But I am happy make the name change to SpanAttributes if the potential conflict/confusion is of no concern.

…tom one

packages/opentelemetry-instrumentation-fetch/src/fetch.ts

…ator # Conflicts: # packages/opentelemetry-plugin-grpc-js/src/client/utils.ts # packages/opentelemetry-plugin-grpc-js/src/server/clientStreamAndUnary.ts # packages/opentelemetry-plugin-grpc-js/src/server/patchServer.ts # packages/opentelemetry-plugin-grpc-js/src/server/serverStreamAndBidi.ts # packages/opentelemetry-plugin-grpc/src/grpc.ts # packages/opentelemetry-plugin-http/src/utils.ts # packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts # packages/opentelemetry-plugin-http/test/functionals/utils.test.ts # packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts # packages/opentelemetry-plugin-http/test/utils/assertSpan.ts # packages/opentelemetry-plugin-https/test/functionals/https-enable.test.ts # packages/opentelemetry-plugin-https/test/integrations/https-enable.test.ts # packages/opentelemetry-plugin-https/test/utils/assertSpan.ts

obecny

Something is wrong during merging with main branch you have restored all the removed plugins

weyert · 2021-04-09T17:10:06Z

Something is wrong during merging with main branch you have restored all the removed plugins

Yes, sorry about that, I noticed that too. Think it's resolved now. Sourcetree is confusing with displaying deleted files when you have changes to the same files

scripts/semconv/templates/SemanticAttributes.ts.j2

obecny

works like a charm, one small suggestion

scripts/semconv/generate.sh

weyert · 2021-04-10T19:35:41Z

Thank you for approving :D

tapico-weyert added 4 commits April 8, 2021 02:10

feat: update semantic-conventations-package by generating using new…

4724305

… build script

fix: update packages to use the new SemanticAttribute-class

310d124

Updates the different packages to fix the breaking change were there is only `SemanticAttribute` and `ResourceAttribute`-class defined in the `@opentelemetry/semantic-conventations`-package

fix: improve the semconv-template to better match eslint prettier r…

5b40934

…ules

weyert requested review from dyladan, Flarna, johnbley, legendecas, markwolff, mayurkale22, mwear, naseemkullah, obecny, OlivierAlbertini and vmarchaud as code owners April 8, 2021 01:26

tapico-weyert added 3 commits April 8, 2021 02:39

chore: run lint:fix-task after generating source files

3e9f64e

chore: move argument to its own line in generate.sh

3302972

vmarchaud reviewed Apr 8, 2021

View reviewed changes

tapico-weyert added 4 commits April 8, 2021 19:01

chore: merge branch 'main' into chore-otel-add-semconv-generator

3ff5b47

style: ran npm run lint:fix on the code base

a8b4115

fix: use RPC_GRPC_STATUS_CODE from semantic-convention that a cus…

c2fb34a

…tom one

test: fix Span test to ensure attribute value is a string

4b16cff

dyladan reviewed Apr 8, 2021

View reviewed changes

packages/opentelemetry-instrumentation-fetch/src/fetch.ts Outdated Show resolved Hide resolved

dyladan mentioned this pull request Apr 8, 2021

Remove obsolete span attributes #2089

Open

dyladan approved these changes Apr 8, 2021

View reviewed changes

vmarchaud approved these changes Apr 8, 2021

View reviewed changes

obecny suggested changes Apr 9, 2021

View reviewed changes

chore: attempt to correct fix failed merge

8eda347

srikanthccv reviewed Apr 9, 2021

View reviewed changes

scripts/semconv/templates/SemanticAttributes.ts.j2 Outdated Show resolved Hide resolved

obecny approved these changes Apr 9, 2021

View reviewed changes

scripts/semconv/generate.sh Outdated Show resolved Hide resolved

tapico-weyert and others added 3 commits April 10, 2021 02:22

fix: remove unwanted @deprecated-attribute in semconv template file

5b45d5b

fix: don't fetch the whole otel specification repo

50ce031

Merge branch 'main' into chore-otel-add-semconv-generator

628bb71

vmarchaud merged commit 031b0f4 into open-telemetry:main Apr 10, 2021

weyert deleted the chore-otel-add-semconv-generator branch April 10, 2021 19:35

vmarchaud added the breaking label Apr 11, 2021

dyladan mentioned this pull request Apr 13, 2021

chore: 0.19.0 release proposal #2107

Merged

DTPierreGutierrez mentioned this pull request May 29, 2024

[Snyk] Upgrade @opentelemetry/context-zone from 0.11.0 to 0.26.0 DTPierreGutierrez/opentelemetry-examples#5

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add semconv generator for `semantic-conventions`-package #2083

feat: add semconv generator for `semantic-conventions`-package #2083

weyert commented Apr 8, 2021 •

edited

Loading

codecov bot commented Apr 8, 2021 •

edited

Loading

vmarchaud Apr 8, 2021

weyert Apr 8, 2021

Oberon00 Apr 8, 2021

dyladan Apr 8, 2021

weyert Apr 8, 2021

weyert commented Apr 8, 2021

dyladan commented Apr 8, 2021

weyert commented Apr 8, 2021

obecny left a comment

weyert commented Apr 9, 2021

obecny left a comment

weyert commented Apr 10, 2021


		{%- if class == "SemanticAttribute" %}

		// Manually defined and not YET in the YAML

feat: add semconv generator for semantic-conventions-package #2083

feat: add semconv generator for semantic-conventions-package #2083

Conversation

weyert commented Apr 8, 2021 • edited Loading

Which problem is this PR solving?

Short description of the changes

codecov bot commented Apr 8, 2021 • edited Loading

Codecov Report

vmarchaud Apr 8, 2021

Choose a reason for hiding this comment

weyert Apr 8, 2021

Choose a reason for hiding this comment

Oberon00 Apr 8, 2021

Choose a reason for hiding this comment

dyladan Apr 8, 2021

Choose a reason for hiding this comment

weyert Apr 8, 2021

Choose a reason for hiding this comment

weyert commented Apr 8, 2021

dyladan commented Apr 8, 2021

weyert commented Apr 8, 2021

obecny left a comment

Choose a reason for hiding this comment

weyert commented Apr 9, 2021

obecny left a comment

Choose a reason for hiding this comment

weyert commented Apr 10, 2021

feat: add semconv generator for `semantic-conventions`-package #2083

feat: add semconv generator for `semantic-conventions`-package #2083

weyert commented Apr 8, 2021 •

edited

Loading

codecov bot commented Apr 8, 2021 •

edited

Loading