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

fix(instrumentation-dns): fix instrumentation of dns/promises #1377

Merged
merged 9 commits into from
Feb 13, 2023

Conversation

mhassan1
Copy link
Contributor

@mhassan1 mhassan1 commented Feb 6, 2023

Which problem is this PR solving?

This PR fixes @opentelemetry/instrumentation-dns when used with dns/promises.

Short description of the changes

This PR adds logic to @opentelemetry/instrumentation-dns that instruments dns/promises. This will properly instrument lookup when it comes from dns/promises; currently, that instrumentation is not happening.

@mhassan1 mhassan1 requested a review from a team February 6, 2023 18:52
@codecov
Copy link

codecov bot commented Feb 6, 2023

Codecov Report

Merging #1377 (1c22d54) into main (00cc8d6) will decrease coverage by 0.64%.
The diff coverage is 9.09%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1377      +/-   ##
==========================================
- Coverage   96.10%   95.47%   -0.64%     
==========================================
  Files          14       18       +4     
  Lines         898     1082     +184     
  Branches      192      217      +25     
==========================================
+ Hits          863     1033     +170     
- Misses         35       49      +14     
Impacted Files Coverage Δ
...lemetry-instrumentation-dns/src/instrumentation.ts 83.54% <9.09%> (ø)
...ode/opentelemetry-instrumentation-dns/src/utils.ts 100.00% <0.00%> (ø)
...ry-instrumentation-dns/src/enums/AttributeNames.ts 100.00% <0.00%> (ø)
...tapackages/auto-instrumentations-node/src/utils.ts 98.21% <0.00%> (ø)

@blumamir
Copy link
Member

BTW, the PR description mentions that currently, we get a "no original function lookup to wrap" warning. Why is that?

@mhassan1
Copy link
Contributor Author

BTW, the PR description mentions that currently, we get a "no original function lookup to wrap" warning. Why is that?

You're right, that's incorrect. That warning was happening on an old version (0.34.0) of @opentelemetry/instrumentation. I will fix the description.

@blumamir blumamir merged commit 6d08157 into open-telemetry:main Feb 13, 2023
@dyladan dyladan mentioned this pull request Feb 13, 2023
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.

2 participants