http instrumentation: url and host attributes are sometime wrong for outgoing requests with ipv6 address

[blumamir]

this is an issue found by @Flarna in this PR

What version of OpenTelemetry are you using?

v0.23.0

What version of Node are you using?

v12.18.3

Please provide the code you used to setup the OpenTelemetry SDK

import express from 'express';
const app = express();
app.use((_req, res) => res.sendStatus(200));
app.listen(8080, '::', () => console.log('server started on port 8080'));

import { NodeTracerProvider } from '@opentelemetry/node';
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { registerInstrumentations } from '@opentelemetry/instrumentation';
import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing';
const provider = new NodeTracerProvider();
provider.addSpanProcessor( new SimpleSpanProcessor(new ConsoleSpanExporter()));
provider.register();
registerInstrumentations({
    instrumentations: [ new HttpInstrumentation() ],
});

import http from 'http';
http.get({host: '::1', port: 8080}, (res) => {
    res.on('data', () => console.log('got data'));
    res.on('end', () => console.log('res end'));
});

What did you do?

run the above code

If possible, provide a recipe for reproducing the error.

What did you expect to see?

expected attributes for outgoing http span: "http.url": "http://[::1]:8080/" (or "http://::1:8080", not sure) "http:host": "[::1]:8080" (or "::1:8080", not sure)

What did you see instead?

{
  traceId: 'cdd140118cdfc23da00059c291a18b7a',
  parentId: undefined,
  name: 'HTTP GET',
  id: '9aeb488a5ff193b6',
  kind: 2,
  timestamp: 1626344909735352,
  duration: 29152,
  attributes: {
    'http.url': 'http://::1/',
    'http.method': 'GET',
    'http.target': '/',
    'net.peer.name': ':',
    'net.peer.ip': '::1',
    'net.peer.port': 8080,
    'http.host': '::8080',
    'http.response_content_length_uncompressed': 2,
    'http.status_code': 200,
    'http.status_text': 'OK',
    'http.flavor': '1.1',
    'net.transport': 'ip_tcp'
  },
  status: { code: 1 },
  events: []
}

As can be seen, "http.url" is missing the port part, and "http.host" is missing the "1" for "::1" host.

Additional context

This happens as the instrumentation is executing the following code on the host options, which is buggy for ipv6:

optionsParsed.host?.replace(/^(.*)(:[0-9]{1,5})/, '$1')

Thanks @Flarna for noticing this problem

[mustafain117]

Part of the issue here is the incorrect RequestOptions parameter for the http.get() function call. The host '::1' is actually just the hostname, changing the get request to :

http.get({hostname: '::1', port: 8080}, (res) => {
  res.on('data', () => console.log('got data'));
  res.on('end', () => console.log('res end'));
});

produces the following:

{
  traceId: '0f55b9a3fd4c2d442af22bb1a07198ef',
  parentId: undefined,
  name: 'HTTP GET',
  id: '8ee9db7b50474589',
  kind: 2,
  timestamp: 1632871597376919,
  duration: 25447,
  attributes: {
    'http.url': 'http://::1/',
    'http.method': 'GET',
    'http.target': '/',
    'net.peer.name': '::1',
    'net.peer.ip': '::1',
    'net.peer.port': 8080,
    'http.host': '::1:8080',
    'http.response_content_length_uncompressed': 2,
    'http.status_code': 200,
    'http.status_text': 'OK',
    'http.flavor': '1.1',
    'net.transport': 'ip_tcp'
  },
  status: { code: 1 },
  events: []
}

The "http.url" is still incorrect however "http.host" is as expected.

"http.url" does not have the port appended due to the following condition which fails for ipv6 hostnames https://github.com/open-telemetry/opentelemetry-js/blob/be52259d736d6762f83d89682dc528c2db0ffeaa/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts#L58

  if (
    (host as string).indexOf(':') === -1 &&
    port &&
    port !== '80' &&
    port !== '443'
  ) {
    host += `:${port}`;
  }

[blumamir]

Part of the issue here is the incorrect RequestOptions parameter for the http.get() function call. The host '::1' is actually just the hostname, changing the get request to :

I agree that it's most appropriate to be set on hostname and port. However, user can still use the above options configuration and node's http implementation will work correctly in this case, so I would expect that the instrumentation should work in this case too.

Node documentation is not so clear about it either:

host <string> A domain name or IP address of the server to issue the request to. Default: 'localhost'.
hostname <string> Alias for host. To support url.parse(), hostname will be used if both host and hostname are specified.

From the docs, I can understand that setting hostname (or domain name, or IP address) on the host options is a legitimate and supported way to configure the client.

[github-actions[bot]]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[dyladan]

I was going to suggest that we use the URL class to parse these attributes out but it seems like it would actually fail with that input even though node http library handles it properly:

> new URL({ host: '::1', port: 8080 })
Uncaught TypeError [ERR_INVALID_URL]: Invalid URL: [object Object]
    at new NodeError (internal/errors.js:322:7)
    at onParseError (internal/url.js:270:9)
    at new URL (internal/url.js:346:5)
    at REPL3:1:1
    at Script.runInThisContext (vm.js:134:12)
    at REPLServer.defaultEval (repl.js:566:29)
    at bound (domain.js:421:15)
    at REPLServer.runBound [as eval] (domain.js:432:12)
    at REPLServer.onLine (repl.js:909:10)
    at REPLServer.emit (events.js:412:35) {
  input: '[object Object]',
  code: 'ERR_INVALID_URL

[github-actions[bot]]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions[bot]]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions[bot]]

This issue was closed because it has been stale for 14 days with no activity.