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

null port when using ignore matchers in opentelemetry-instrumentation-http #2947

Closed
danielgblanco opened this issue May 3, 2022 · 2 comments · Fixed by #2948
Closed

null port when using ignore matchers in opentelemetry-instrumentation-http #2947

danielgblanco opened this issue May 3, 2022 · 2 comments · Fixed by #2948
Assignees
Labels
bug Something isn't working

Comments

@danielgblanco
Copy link
Contributor

danielgblanco commented May 3, 2022

What version of OpenTelemetry are you using?

npm list
[email protected] ~/test
├── @opentelemetry/[email protected]
├── @opentelemetry/[email protected]
├── @opentelemetry/[email protected]
└── [email protected]

What version of Node are you using?

node --version
v16.14.0

Please provide the code you used to setup the OpenTelemetry SDK

const { NodeTracerProvider } =  require('@opentelemetry/sdk-trace-node');
const {HttpInstrumentation} = require('@opentelemetry/instrumentation-http');

const instrumentation = new HttpInstrumentation();
const provider = new NodeTracerProvider();
instrumentation.setTracerProvider(provider);
instrumentation.enable();

const axios = require('axios');

const ignoreMatcher = (url) => {
  console.info(`${typeof url}: ${url}`)
  return new URL(url).hostname.endsWith('example.com');
}

instrumentation.setConfig({
  ignoreOutgoingUrls: [
    ignoreMatcher
  ],
});

What did you do?

axios.get(
  'http://example.com',
).then();

What did you expect to see?

string: http://example.com/

What did you see instead?

string: http://example.com:null/

Additional context

There seems to be an issue in the opentelemetry-instrumentation-http when getting request info on a request passed as a string to Axios on default port. The code here expects a port but the options object looks something like.

{
  "maxRedirects": 21,
  "maxBodyLength": 10485760,
  "protocol": "http:",
  "path": "/",
  "method": "GET",
  "headers": {
    "Accept": "application/json, text/plain, */*",
    "User-Agent": "axios/0.27.2"
  },
  "agents": {},
  "hostname": "example.com",
  "port": null,
  "nativeProtocols": {...},
  "pathname": "/"
}

This is not a critical issue, but it means that origin passed to isIgnored here may not be correctly formatted, thus potentially resulting in exceptions thrown (when trying to cast into URL) or matching not occurring (if trying to match a string).

@danielgblanco danielgblanco added the bug Something isn't working label May 3, 2022
danielgblanco added a commit to danielgblanco/opentelemetry-js that referenced this issue May 3, 2022
danielgblanco added a commit to danielgblanco/opentelemetry-js that referenced this issue May 3, 2022
danielgblanco added a commit to danielgblanco/opentelemetry-js that referenced this issue May 3, 2022
@dyladan
Copy link
Member

dyladan commented May 4, 2022

Assigned you since you're already working here

@danielgblanco
Copy link
Contributor Author

Thanks, yes the PR linked above should close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants