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

chore(detectors/node): adapted breaking change in @opentelemetry/sdk-node@0.37.0 #1462

Merged
merged 3 commits into from
Apr 12, 2023

Conversation

kirrg001
Copy link
Contributor

@kirrg001 kirrg001 commented Apr 6, 2023

Refs #1459 (comment)

I do no have write permissions for renovate-bot:renovate/otel-core-experimental.
Feel free to close this PR and cherry-pick the commit.

The README files for all detectors needs to be updated too. Let me know if you want me to update them too.

@codecov
Copy link

codecov bot commented Apr 6, 2023

Codecov Report

Merging #1462 (d7364fc) into main (27943f4) will increase coverage by 0.35%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1462      +/-   ##
==========================================
+ Coverage   96.13%   96.49%   +0.35%     
==========================================
  Files          14        2      -12     
  Lines         906       57     -849     
  Branches      197        9     -188     
==========================================
- Hits          871       55     -816     
+ Misses         35        2      -33     
Impacted Files Coverage Δ
...ctor-instana/src/detectors/InstanaAgentDetector.ts 96.42% <100.00%> (ø)

... and 15 files with indirect coverage changes

@Flarna
Copy link
Member

Flarna commented Apr 6, 2023

An alternative would be that you cherry pick the commit from renovate and add you fix on top of it.
But this variant is also fine. Once merged renovate will rebase on top of your fix and CI is hopefully green then.

@Flarna
Copy link
Member

Flarna commented Apr 11, 2023

@kirrg001 It seems you didn't allow edits from maintainers on this branch - or it's an org fork where github doesn't allow this (see here).

So seems the burden to merge main is on you.

…node@0.37.0

- open-telemetry/opentelemetry-js#3460
- Deprecated detectResources() in favor of a new method detectResourcesSync() which defers promises into the resource to be resolved later
- introduced await resource.waitForAsyncAttributes?.();
@kirrg001
Copy link
Contributor Author

or it's an org fork where github doesn't allow this

Yes it is an organization. This is not supported.
I rebased against main. Rdy to merge

@Flarna
Copy link
Member

Flarna commented Apr 12, 2023

At least in the past it was possible to allow it per PR, see here.

In general I recommend a personal fork but I know that companies often don't allow this.

@Flarna Flarna merged commit d2d8288 into open-telemetry:main Apr 12, 2023
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.

None yet

4 participants