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

[SDK 2.0] merge changes from next to main #5284

Merged

Conversation

pichlermarc
Copy link
Member

@pichlermarc pichlermarc commented Dec 18, 2024

Description

Important

This PR needs to be merged by a maintainer as we need to merge it non-squashed. Otherwise we'll not have the full history from next on main.

Updates main with the latest changes from next, all changes here are pre-reviewed already, except for the changes to

  • README.md (manually merged from README_NEXT.md into README.md, adapted warning about this being the 2.0 development branch).
  • CHANGELOG.md (manually merged from CHANGELOG_NEXT.md into CHANGELOG.md).
    v1.x develoment will continue through bugfix backports and security fixes only on the v1.x branch.

Tracking issue #5148

dyladan and others added 30 commits November 15, 2023 11:15
* chore: track package-lock.json (open-telemetry#4238)

* chore: track package-lock.json

* Pin to old versions for node 14

* Use version range

* Remove unused cached directories

* Temporarily disable other tests

* Temporarily enable only api test

* Enable only some packages

* Test only api packages

* Test trace exporters

* Fix line ordering

* Test all packages except otlp exporters

* Add trace http exporter

* Add trace proto exporter

* Test all but grpc exporters

* chore: use npm workspaces and degrade lerna to v6

* chore: get rid of lerna bootstrap

* chore: use npx

* chore: allow install scripts to setup buf

* chore: fix w3c-integration-test cache key

* chore: fix cache key

* chore: disable resource compat test

* chore: fix node_modules assumptions

* chore: fix hoisted karma issue

* chore: fix markdown linter complaints

* chore: lock @grpc/grpc-js to v1.8.21

* Break caches

* chore: remove cache

* chore: fixup inline commands

---------

Co-authored-by: Daniel Dyla <[email protected]>

* docs: fixed link to benchmark results (open-telemetry#4233)

Co-authored-by: Chengzhong Wu <[email protected]>

* chore(deps): update all patch versions (open-telemetry#4215)

* fix: otlp json encoding (open-telemetry#4220)

Co-authored-by: Marc Pichler <[email protected]>

* fix: remove duplicate export star from version.ts (open-telemetry#4225)

Co-authored-by: Marc Pichler <[email protected]>

* docs: fix sdk-node config instructions (open-telemetry#4249)

Co-authored-by: Marc Pichler <[email protected]>

* feat(api): publish api esnext target (open-telemetry#4231)

* chore: release API 1.7.0/Core 1.18.0/Experimental 0.45.0 (open-telemetry#4254)

* fix(sdk-metrics): hand-roll MetricAdvice type as older API versions do not include it (open-telemetry#4260)

* chore: prepare release 1.18.1/0.45.1 (open-telemetry#4261)

* chore: no need for 'packages' in "lerna.json" (open-telemetry#4264)

* Benchmark tests for trace OTLP transform and BatchSpanProcessor (open-telemetry#4218)

Co-authored-by: Marc Pichler <[email protected]>

* chore: type reference on zone.js (open-telemetry#4257)

Co-authored-by: Marc Pichler <[email protected]>

* docs: add docker-compose to run prometheus for the experimental example (open-telemetry#4268)

Co-authored-by: Marc Pichler <[email protected]>

* fix(sdk-logs): avoid map attribute set when count limit exceeded (open-telemetry#4195)

Co-authored-by: Marc Pichler <[email protected]>

* chore(deps): update dependency chromedriver to v119 [security] (open-telemetry#4280)

* chore(deps): update actions/setup-node action to v4 (open-telemetry#4236)

* fix(sdk-trace-base): processor onStart called with a span having empty attributes (open-telemetry#4277)

Co-authored-by: artahmetaj <[email protected]>

* Update fetch instrumentation to be runtime agnostic (open-telemetry#4063)

Co-authored-by: Marc Pichler <[email protected]>

---------

Co-authored-by: Chengzhong Wu <[email protected]>
Co-authored-by: Martin Kuba <[email protected]>
Co-authored-by: Mend Renovate <[email protected]>
Co-authored-by: Siim Kallas <[email protected]>
Co-authored-by: Marc Pichler <[email protected]>
Co-authored-by: David Luna <[email protected]>
Co-authored-by: Dinko Osrecki <[email protected]>
Co-authored-by: Trent Mick <[email protected]>
Co-authored-by: François <[email protected]>
Co-authored-by: Hyun Oh <[email protected]>
Co-authored-by: André Cruz <[email protected]>
Co-authored-by: artahmetaj <[email protected]>
Co-authored-by: drewcorlin1 <[email protected]>
* Add Trent to approvers (open-telemetry#4311)

* chore(renovate): require dashboard approval for lerna updates (open-telemetry#4276)

* chore(ci): install semver globally to speed up "peer-api" workflow (open-telemetry#4270)

Closes: open-telemetry#4242

* fix(ci): remove token setup via environment variable from .npmrc (open-telemetry#4329)

* fix(instrumentation-http): resume responses when there is no response listener

Fixes a memory leak where unhandled response bodies pile up in node 20

* feat: add script to update changelogs on release preparation (open-telemetry#4315)

* feat: add script to update changelogs on releases

* fix: address comments

* Apply suggestions from code review

Co-authored-by: Trent Mick <[email protected]>

* fix: apply suggestions from code review

* fix: use packageJson.version instead of version

---------

Co-authored-by: Trent Mick <[email protected]>

* Fix event name

* test: make rawRequest HTTP-compliant

* Add node 20 to test matrix

* Enable old hash functions on 20

* Fix esm handling for iitm node 20

* Use err.code to make test more reliable

* Changelog

* nit: single import

* Remove unused files

* Add v20 to supported runtimes

* ci: add npm cache in actions/setup-node (open-telemetry#4271)

* feat(sdk-logs): add droppedAttributesCount field to ReadableLogRecord (open-telemetry#4289)

* feat(sdk-logs): add droppedAttributesCount field to ReadableLogRecord

* chore: check droppedAttributesCount value in test case

* feat(otlp-transformer): make toLogRecord() use ReadableLogRecord.droppedAttributesCount

---------

Co-authored-by: Marc Pichler <[email protected]>

* fix(api-logs): allow passing in TimeInput for LogRecord (open-telemetry#4345)

* fix: allow passing in TimeInput for LogRecord

* chore: update changelog

* fix: programmatic url and headers take precedence in metric exporters… (open-telemetry#4334)

* fix: programmatic url and headers take precedence in metric exporters (open-telemetry#2370)

* chore: adjust grpc exporter metrics test

* chore(changelog): update changelog

* fix(instrumentation-http): do not mutate given headers object for outgoing http requests (open-telemetry#4346)

Fixes: open-telemetry/opentelemetry-js-contrib#1609

* chore(deps): update actions/stale action to v9 (open-telemetry#4353)

* fix(deps): update dependency import-in-the-middle to v1.6.0 (open-telemetry#4357)

* chore(deps): update all patch versions (open-telemetry#4306)

* chore(ci): use node 20 in lint workflow (open-telemetry#4359)

* chore(deps): update dependency linkinator to v6 (open-telemetry#4237)

* fix(otlp-exporter-base): decrease default concurrency limit to 30 (open-telemetry#4211)

* fix(otlp-exporter-base): decrease concurrency limit to 30

* fix(changelog): add changelog entry

* chore(deps): use actions/checkout >4 instead of 4.0.0 exactly (open-telemetry#4361)

---------

Co-authored-by: Marc Pichler <[email protected]>
Co-authored-by: strivly <[email protected]>
Co-authored-by: Trent Mick <[email protected]>
Co-authored-by: lyzlisa <[email protected]>
Co-authored-by: Hyun Oh <[email protected]>
Co-authored-by: Siim Kallas <[email protected]>
Co-authored-by: Vladimir Adamić <[email protected]>
Co-authored-by: Mend Renovate <[email protected]>
* chore: remove checks for unsupported versions

* Remove final version checks

* Changelog

* Lint
)

* chore(otel-core): replace deprecated spanAttributes

* update changelog

* keep new line at bottom of package.json
…metry#4430)

* chore(shim-opentracing): replace deprecated spanAttributes

* update minimum api version to 1.1

* keep newline in package.json

* update changelog
…try#4428)

* chore(otel-resources): replace deprecated spanAttributes

* update minimum api version to 1.1

* update changelog

* per legendecas, add todo for ResourceAttributes

---------

Co-authored-by: Marc Pichler <[email protected]>
…f constructor option (open-telemetry#4419)

* feat(sdk-metrics)!: remove MeterProvider.addMetricReader() in favor of constructor option

* fix(changelog): add changelog entry
…t-main-02-09

 `[next]` merge changes from `main`
@pichlermarc pichlermarc requested a review from a team as a code owner December 18, 2024 16:40
README.md Outdated
Comment on lines 38 to 41
> [!WARNING]
> This is the working branch for the work in progress 2.0 SDK, see [this tracking issue](https://github.com/open-telemetry/opentelemetry-js/issues/5148) for details.
> If you are a user looking for the currently released state, you are probably looking for the [1.x SDK](https://github.com/open-telemetry/opentelemetry-js/tree/v1.x) on the v1.x branch.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only change to README.md that has not been reviewed already.

Comment on lines +14 to +27
* feat(sdk-metrics)!: bump minimum version of `@opentelemetry/api` peer dependency to 1.9.0 [#5254](https://github.com/open-telemetry/opentelemetry-js/pull/5254) @chancancode
* chore(shim-opentracing): replace deprecated SpanAttributes [#4430](https://github.com/open-telemetry/opentelemetry-js/pull/4430) @JamieDanielson
* chore(otel-core): replace deprecated SpanAttributes [#4408](https://github.com/open-telemetry/opentelemetry-js/pull/4408) @JamieDanielson
* feat(sdk-metrics)!: remove MeterProvider.addMetricReader() in favor of constructor option [#4419](https://github.com/open-telemetry/opentelemetry-js/pull/4419) @pichlermarc
* chore(otel-resources): replace deprecated SpanAttributes [#4428](https://github.com/open-telemetry/opentelemetry-js/pull/4428) @JamieDanielson
* feat(sdk-metrics)!: remove MeterProvider.addMetricReader() in favor of constructor option [#4419](https://github.com/open-telemetry/opentelemetry-js/pull/4419) @pichlermarc
* feat(sdk-metrics)!: replace attributeKeys with custom processors option [#4532](https://github.com/open-telemetry/opentelemetry-js/pull/4532) @pichlermarc
* refactor(sdk-trace-base)!: replace `SpanAttributes` with `Attributes` [#5009](https://github.com/open-telemetry/opentelemetry-js/pull/5009) @david-luna
* refactor(resources)!: replace `ResourceAttributes` with `Attributes` [#5016](https://github.com/open-telemetry/opentelemetry-js/pull/5016) @david-luna
* feat(sdk-metrics)!: drop `View` and `Aggregation` in favor of `ViewOptions` and `AggregationOption` [#4931](https://github.com/open-telemetry/opentelemetry-js/pull/4931) @pichlermarc
* refactor(sdk-trace-base)!: remove `new Span` constructor in favor of `Tracer.startSpan` API [#5048](https://github.com/open-telemetry/opentelemetry-js/pull/5048) @david-luna
* refactor(sdk-trace-base)!: remove `BasicTracerProvider.addSpanProcessor` API in favor of constructor options. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5134) @david-luna
* refactor(sdk-trace-base)!: make `resource` property private in `BasicTracerProvider` and remove `getActiveSpanProcessor` API. [#5192](https://github.com/open-telemetry/opentelemetry-js/pull/5192) @david-luna

Copy link
Member Author

Choose a reason for hiding this comment

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

copied from CHANGELOG_NEXT.md, and then I deleted CHANGELOG_NEXT.md

Comment on lines +36 to +39
* refactor(sdk-metrics): remove `Gauge` and `MetricAdvice` workaround types in favor of the upstream `@opentelemetry/api` types [#5254](https://github.com/open-telemetry/opentelemetry-js/pull/5254) @chancancode
* chore: remove checks for unsupported node versions [#4341](https://github.com/open-telemetry/opentelemetry-js/pull/4341) @dyladan
* refactor(sdk-trace-base): remove `BasicTracerProvider._registeredSpanProcessors` private property. [#5134](https://github.com/open-telemetry/opentelemetry-js/pull/5134) @david-luna
* refactor(sdk-trace-base): rename `BasicTracerProvider.activeSpanProcessor` private property. [#5211](https://github.com/open-telemetry/opentelemetry-js/pull/5211) @david-luna
Copy link
Member Author

Choose a reason for hiding this comment

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

copied from CHANGELOG_NEXT.md, and then I deleted CHANGELOG_NEXT.md

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

one small nit left, otherwise LGTM
Thanks for working on this!

Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 96.55172% with 6 lines in your changes missing coverage. Please review.

Project coverage is 94.63%. Comparing base (616d27a) to head (2610521).
Report is 65 commits behind head on main.

Files with missing lines Patch % Lines
packages/sdk-metrics/src/view/AggregationOption.ts 87.50% 4 Missing ⚠️
...try-resources/src/detectors/BrowserDetectorSync.ts 0.00% 1 Missing ⚠️
...ors/platform/node/ServiceInstanceIdDetectorSync.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5284      +/-   ##
==========================================
- Coverage   94.66%   94.63%   -0.04%     
==========================================
  Files         322      323       +1     
  Lines        8058     8084      +26     
  Branches     1632     1643      +11     
==========================================
+ Hits         7628     7650      +22     
- Misses        430      434       +4     
Files with missing lines Coverage Δ
...er-metrics-otlp-http/src/OTLPMetricExporterBase.ts 89.23% <100.00%> (+0.16%) ⬆️
...etry-exporter-prometheus/src/PrometheusExporter.ts 96.34% <100.00%> (+0.04%) ⬆️
...imental/packages/opentelemetry-sdk-node/src/sdk.ts 96.96% <100.00%> (ø)
...ckages/opentelemetry-core/src/common/attributes.ts 93.02% <100.00%> (ø)
...metry-core/src/trace/sampler/ParentBasedSampler.ts 83.87% <ø> (ø)
packages/opentelemetry-resources/src/Resource.ts 100.00% <100.00%> (ø)
...lemetry-resources/src/detectors/EnvDetectorSync.ts 94.11% <100.00%> (ø)
...es/src/detectors/platform/node/HostDetectorSync.ts 100.00% <100.00%> (ø)
...rces/src/detectors/platform/node/OSDetectorSync.ts 100.00% <100.00%> (ø)
...src/detectors/platform/node/ProcessDetectorSync.ts 92.85% <100.00%> (ø)
... and 23 more

... and 1 file with indirect coverage changes

@pichlermarc
Copy link
Member Author

I removed the required checks for Node.js v14 and Node.js v16 since we dropped support for it.

Copy link
Member

@dyladan dyladan left a comment

Choose a reason for hiding this comment

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

This is a lot... but i think it all looks good to me. There's no great way to review a 3000 line diff that touches 146 files 🤣

@pichlermarc
Copy link
Member Author

pichlermarc commented Dec 18, 2024

This is a lot... but i think it all looks good to me. There's no great way to review a 3000 line diff that touches 146 files 🤣

True - for future reference: all of these commits that are merged here from next went through the proper review process that matches the one on main. So the reveiew in this case would consist mostly just verifying that I did not add anything extra that should not be there - one "okay" way would be to look at the diff for the head of this branch and next - that diff should just match my last commit here (2610521), and then just reviewing that diff. 🙂

@@ -9,6 +9,7 @@ on:
types: [opened, synchronize, reopened, labeled, unlabeled]
branches:
- main
- next
Copy link
Contributor

Choose a reason for hiding this comment

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

Eventually we'll be able to drop this change, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes - I'll follow up with a PR to clean this up :)

Copy link
Member

Choose a reason for hiding this comment

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

I think we likely could drop it now. The next branch will go away

Copy link
Contributor

Choose a reason for hiding this comment

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

@pichlermarc when you get around to this, PRs to the v1.x maintenance branch no longer triggers this workflow. As the two branches diverge over time, there would probably be some PRs that needs to target that branch once in a while. I would fix it, but I can never remember whether you are supposed to change the workflow file on main or the one on the branch :)

@pichlermarc pichlermarc added this pull request to the merge queue Dec 18, 2024
Merged via the queue into open-telemetry:main with commit 107637e Dec 18, 2024
18 checks passed
@pichlermarc pichlermarc deleted the next-main-2024-12-16 branch December 18, 2024 19:10
@dyladan
Copy link
Member

dyladan commented Dec 18, 2024

This is a lot... but i think it all looks good to me. There's no great way to review a 3000 line diff that touches 146 files 🤣

True - for future reference: all of these commits that are merged here from next went through the proper review process that matches the one on main. So the reveiew in this case would consist mostly just verifying that I did not add anything extra that should not be there - one "okay" way would be to look at the diff for the head of this branch and next - that diff should just match my last commit here (2610521), and then just reviewing that diff. 🙂

thats basically what i did. I also looked for things like merge conflict markers, but you'd expect most basic merge conflicts to break the build anyway so i'm reasonably confident

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.

9 participants