Skip to content

work around IPv6 addresses in Trace#fromRequest #35

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

work around IPv6 addresses in Trace#fromRequest #35

wants to merge 1 commit into from

Conversation

flugeldoo
Copy link

When tracing systems on IPv6 networks, transmission to the collector fails.

error:  Error: IPv4 string does not have 4 parts.
    at ipv4ToNumber (.../node_modules/tryfer/lib/formatters.js:123:13)
    at .../node_modules/tryfer/lib/formatters.js:157:17
    at Array.forEach (native)
    at Object.formatForZipkin (.../node_modules/tryfer/lib/formatters.js:153:29)
    at RawZipkinTracer._sendTrace (.../node_modules/tryfer/lib/node_tracers.js:219:14)
    at replenish (.../node_modules/tryfer/node_modules/async/lib/async.js:144:17)
    at Object.async.forEachLimit (.../node_modules/tryfer/node_modules/async/lib/async.js:161:11)
    at RawZipkinTracer.sendTraces (.../node_modules/tryfer/lib/node_tracers.js:230:9)
    at BufferingTracer._sendTraces (.../node_modules/tryfer/lib/node_tracers.js:100:16)
    at BufferingTracer._periodicSendFunction (.../node_modules/tryfer/lib/node_tracers.js:90:10)
    at Timer.listOnTimeout (timers.js:119:15)

Zipkin's Thrift schema doesn't accommodate IPv6 addresses, so the only way to deal with them is to mask them.

@suryatech
Copy link
Contributor

@niccaluim Can you please fix the lint errors that are outlined here?

I am not sure if we should write our own IPv4 validator. Have you considered using a regular expression for validation purposes?

Everything apart from this looks good to me.

@suryatech
Copy link
Contributor

Here's a small perf test

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.

2 participants