Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(semantic-conventions): update semantic conventions to v1.29.0 #5356

Merged
merged 11 commits into from
Feb 6, 2025

Conversation

trentm
Copy link
Contributor

@trentm trentm commented Jan 21, 2025

This also adds a changelog-gen.js script to help generate meaningful summary of changes in the semconv JS package.

Note: This change excludes the new 'network.interface.name' attribute because of open-telemetry/semantic-conventions#1777


Note: I did not update the weaver version to the latest. Doing so includes some whitespace fixes in the generated output that I wanted to do in a separate PR after this one.

This also adds a changelog-gen.js script to help generate meaningful
summary of changes in the semconv JS package.

Note: This change *excludes* the new 'network.interface.name' attribute
because of open-telemetry/semantic-conventions#1777
@trentm trentm self-assigned this Jan 21, 2025
@trentm trentm requested a review from a team as a code owner January 21, 2025 20:32
@trentm
Copy link
Contributor Author

trentm commented Jan 21, 2025

In the last semconv update PR we discussed some generation of better changelogs for semconv updates. This PR adds a script to help with that. The result is in the diff of semantic-conventions/CHANGELOG.md
Here is a link to the rendered version of that:

https://github.com/trentm/opentelemetry-js/blob/tm-semconv-1.29.0/semantic-conventions/CHANGELOG.md#rocket-enhancement

  • Be sure to open the "Full unstable changes" disclosure to see the detailed summary.
  • Currently I setup the generated changelog to put unstable changes behind the disclosure, but stable changes, if any, would not be hidden behind a disclosure. It just so happens that v1.29.0 didn't include any stable changes.

Copy link

codecov bot commented Jan 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.79%. Comparing base (d803022) to head (3463e5d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5356   +/-   ##
=======================================
  Coverage   94.79%   94.79%           
=======================================
  Files         310      310           
  Lines        7974     7974           
  Branches     1682     1682           
=======================================
  Hits         7559     7559           
  Misses        415      415           

@trentm
Copy link
Contributor Author

trentm commented Jan 22, 2025

A comment/suggestion on the changelog entry formatting from an OTel JS maintainers call earlier today: Separate disclosures for each change type. "Added" change type at the bottom.

…have an explicit stability; this is a more general fix to skip the current case (network.interface.name), per suggestion from Liudmila
@trentm
Copy link
Contributor Author

trentm commented Jan 22, 2025

Comment on changelog: separate disclosures for each category. "Added" at the bottom.

Here is a gist showing an example rendering. Note that I've manually faked some of the unstable changes as "stable" changes, so we can see some data in that section. (The actual 1.29.0 change did not include any changes to stable exports.)

https://gist.github.com/trentm/0decada22acd953813efb698a1985f29

In this rendering, both the stable and unstable sections are using the same <details> ... markup -- but the stable section defaults those disclosures being open (<details open> ...).

… to help with grepping); update the changelog-gen script to support comparing older versions (e.g. changelog-gen.js 1.27.0 1.28.0)
Copy link
Member

@JamieDanielson JamieDanielson left a comment

Choose a reason for hiding this comment

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

I love this! I suggested a change to the usage comments, and had a question around usage when the local version is passed into the changelog script, but it's not blocking since I don't see it being a common use case and happy to hear what I did wrong 😄

const isPrevDeprecated = prevNames.has(k) && isDeprecated(prevSrc, k);
const isCurrDeprecated = currNames.has(k) && isDeprecated(currSrc, k);
if (isPrevDeprecated && !isCurrDeprecated) {
throw new Error(`semconv export '${k}' was *un*-deprecated in this release!? Wassup?`);
Copy link
Member

Choose a reason for hiding this comment

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

semconv export '${k}' was *un*-deprecated in this release!? Wassup?

😄

scripts/semconv/changelog-gen.js Outdated Show resolved Hide resolved
scripts/semconv/changelog-gen.js Outdated Show resolved Hide resolved
scripts/semconv/changelog-gen.js Show resolved Hide resolved
trentm and others added 4 commits February 5, 2025 16:42
Co-authored-by: Jamie Danielson <[email protected]>
Co-authored-by: Jamie Danielson <[email protected]>
…p the 'curl | tar' command so that a failed curl actually breaks there with a more meaningful message
@trentm trentm added this pull request to the merge queue Feb 6, 2025
github-merge-queue bot pushed a commit that referenced this pull request Feb 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 6, 2025
@trentm
Copy link
Contributor Author

trentm commented Feb 6, 2025

Merge queue checks failed in shim-opentracing tests for Node v20: https://github.com/open-telemetry/opentelemetry-js/actions/runs/13169707713/job/36757662560

   29 passing (37ms)
  1 failing

  1) OpenTracing Shim
       TracerShim
         starting spans
           translates span options correctly:

      AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected

+ 1738804269038.6545
- 1738804269038.6548
                   ^
      + expected - actual

      -1738804269038.6545
      +1738804269038.6548
      
      at Context.<anonymous> (test/Shim.test.ts:266:16)

@trentm trentm added this pull request to the merge queue Feb 6, 2025
Merged via the queue into open-telemetry:main with commit 2d4e1ca Feb 6, 2025
18 checks passed
@trentm trentm deleted the tm-semconv-1.29.0 branch February 6, 2025 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants