Skip to content

Commit

Permalink
Fix parser defaults (#533)
Browse files Browse the repository at this point in the history 
* docs: add deprecation warnings around trace context parsing
* fix: trace context parsing auto-detection
- do not auto-detect honeycomb or w3c headers if a parser hook is configured
  • Loading branch information
@vreynolds
vreynolds committed Jan 13, 2022
1 parent 14c5198 commit 551a2da
Show file tree
Hide file tree
Showing 6 changed files with 247 additions and 174 deletions.
175 changes: 89 additions & 86 deletions lib/instrumentation/express.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,103 +77,104 @@ function wrapErrMiddleware(middleware, debugWrap) {
};
}

const getMagicMiddleware = ({ userContext, traceIdSource, parentIdSource, packageVersion }) => (
req,
res,
next
) => {
// parse incoming trace headers into a span context
let spanContext = traceUtil.parseTraceHeader(traceIdSource, req);

// parentSpanId refers to the span id of the parent span
let parentSpanId = traceUtil.getParentSourceId(parentIdSource, req);
if (parentSpanId) {
spanContext.parentSpanId = parentSpanId;
}

// propagate trace level fields as-is, if there are any
let propagatedContext = {};
if (spanContext.customContext) {
propagatedContext = spanContext.customContext;
}

let fields = {
[schema.EVENT_TYPE]: "express",
[schema.PACKAGE_VERSION]: packageVersion,
[schema.TRACE_SPAN_NAME]: "request",
[schema.TRACE_ID_SOURCE]: spanContext.source,
"request.host": req.hostname,
"request.original_url": req.originalUrl,
"request.remote_addr": req.ip,
"request.secure": req.secure,
"request.method": req.method,
"request.scheme": req.protocol,
"request.path": req.path,
"request.query": req.query,
"request.http_version": `HTTP/${req.httpVersion}`,
"request.fresh": req.fresh,
"request.xhr": req.xhr,
};

for (let [key, value] of Object.entries(traceUtil.getInstrumentedRequestHeaders())) {
let header = req.get(key);
if (undefined != header) {
fields[value] = header;
const getMagicMiddleware =
({ userContext, traceIdSource, parentIdSource, packageVersion }) =>
(req, res, next) => {
// parse incoming trace headers into a span context
let spanContext = traceUtil.parseTraceHeader(traceIdSource, req);

// parentSpanId refers to the span id of the parent span
let parentSpanId = traceUtil.getParentSourceId(parentIdSource, req);
if (parentSpanId) {
spanContext.parentSpanId = parentSpanId;
}
}

let rootSpan = api.startTrace(
fields,
spanContext.traceId,
spanContext.parentSpanId,
spanContext.dataset,
propagatedContext
);

if (!rootSpan) {
// sampler has decided that we shouldn't trace this request
next();
return;
}
// propagate trace level fields as-is, if there are any
let propagatedContext = {};
if (spanContext.customContext) {
propagatedContext = spanContext.customContext;
}

// we bind the method that finishes the request event so that we're guaranteed to get an event
// regardless of any lapses in context propagation. Doing it this way also allows us to _detect_
// if there was a lapse, since `context` will be undefined in that case.
let boundFinisher = api.bindFunctionToTrace((response, context) => {
if (!context) {
api._askForIssue("we lost our tracking somewhere in the stack handling this request", debug);
let fields = {
[schema.EVENT_TYPE]: "express",
[schema.PACKAGE_VERSION]: packageVersion,
[schema.TRACE_SPAN_NAME]: "request",
[schema.TRACE_ID_SOURCE]: spanContext.source,
"request.host": req.hostname,
"request.original_url": req.originalUrl,
"request.remote_addr": req.ip,
"request.secure": req.secure,
"request.method": req.method,
"request.scheme": req.protocol,
"request.path": req.path,
"request.query": req.query,
"request.http_version": `HTTP/${req.httpVersion}`,
"request.fresh": req.fresh,
"request.xhr": req.xhr,
};

for (let [key, value] of Object.entries(traceUtil.getInstrumentedRequestHeaders())) {
let header = req.get(key);
if (undefined != header) {
fields[value] = header;
}
}

let userEventContext = traceUtil.getUserContext(userContext, req);
if (userEventContext) {
rootSpan.addContext(userEventContext);
let rootSpan = api.startTrace(
fields,
spanContext.traceId,
spanContext.parentSpanId,
spanContext.dataset,
propagatedContext
);

if (!rootSpan) {
// sampler has decided that we shouldn't trace this request
next();
return;
}

rootSpan.addContext({
"request.base_url": req.baseUrl,
"response.status_code": String(response.statusCode),
});
if (req.route) {
// we bind the method that finishes the request event so that we're guaranteed to get an event
// regardless of any lapses in context propagation. Doing it this way also allows us to _detect_
// if there was a lapse, since `context` will be undefined in that case.
let boundFinisher = api.bindFunctionToTrace((response, context) => {
if (!context) {
api._askForIssue(
"we lost our tracking somewhere in the stack handling this request",
debug
);
}

let userEventContext = traceUtil.getUserContext(userContext, req);
if (userEventContext) {
rootSpan.addContext(userEventContext);
}

rootSpan.addContext({
"request.route": req.route.path,
"request.base_url": req.baseUrl,
"response.status_code": String(response.statusCode),
});
}
if (req.params) {
Object.keys(req.params).forEach((param) =>
if (req.route) {
rootSpan.addContext({
[`request.param.${param}`]: req.params[param],
})
);
}

api.finishTrace(rootSpan);
});
"request.route": req.route.path,
});
}
if (req.params) {
Object.keys(req.params).forEach((param) =>
rootSpan.addContext({
[`request.param.${param}`]: req.params[param],
})
);
}

api.finishTrace(rootSpan);
});

onHeaders(res, function () {
return boundFinisher(this, tracker.getTracked());
});
next();
};
onHeaders(res, function () {
return boundFinisher(this, tracker.getTracked());
});
next();
};

let instrumentExpress = function (express, opts = {}) {
let userContext, traceIdSource, parentIdSource;
Expand All @@ -189,6 +190,7 @@ let instrumentExpress = function (express, opts = {}) {
}

if (opts.traceIdSource) {
debug("traceIdSource option is deprecated, please use httpTraceParserHook");
if (typeof opts.traceIdSource === "string" || typeof opts.traceIdSource === "function") {
traceIdSource = opts.traceIdSource;
} else {
Expand All @@ -199,6 +201,7 @@ let instrumentExpress = function (express, opts = {}) {
}

if (opts.parentIdSource) {
debug("parentIdSource option is deprecated, please use httpTraceParserHook");
if (typeof opts.parentIdSource === "string" || typeof opts.traceIdSource === "function") {
parentIdSource = opts.parentIdSource;
} else {
Expand Down
11 changes: 6 additions & 5 deletions lib/instrumentation/trace-util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ function _includeSource(context, key) {
});
}

// in the next major release, consolidate everything around hooks
exports.parseTraceHeader = (traceIdSource, req) => {
let parsed = propagation.parseFromRequest(req);
if (!parsed) {
parsed = exports.getTraceContext(traceIdSource, req);
if (propagation.hasCustomHttpParserHook()) {
let parsed = propagation.parseFromRequest(req);
return parsed ? parsed : {};
}
return parsed;
return exports.getTraceContext(traceIdSource, req);
};

exports.propagateTraceHeader = () => {
Expand All @@ -36,7 +37,7 @@ exports.propagateTraceHeader = () => {
};

// parse a trace header and return object used to populate args to startTrace
// deprecated: in the next major version this should get renamed to getSpanContext
// deprecated: in the next major release convert this to a default parser hook, and update usage logic
exports.getTraceContext = (traceIdSource, req) => {
const { honeycomb, w3c, aws } = api;
if (typeof traceIdSource === "undefined" || typeof traceIdSource === "string") {
Expand Down

0 comments on commit 551a2da

Please sign in to comment.