From 252d357924b94eaaca0e46caf48d8a1aa6356409 Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Wed, 10 May 2023 18:26:28 +0200 Subject: [PATCH 1/2] move http client to clientPlugin --- packages/datadog-plugin-http/src/client.js | 138 +++++++++++---------- 1 file changed, 70 insertions(+), 68 deletions(-) diff --git a/packages/datadog-plugin-http/src/client.js b/packages/datadog-plugin-http/src/client.js index b9620bca7df..e1fbf18a09b 100644 --- a/packages/datadog-plugin-http/src/client.js +++ b/packages/datadog-plugin-http/src/client.js @@ -1,6 +1,6 @@ 'use strict' -const Plugin = require('../../dd-trace/src/plugins/plugin') +const ClientPlugin = require('../../dd-trace/src/plugins/client') const { storage } = require('../../datadog-core') const tags = require('../../../ext/tags') const analyticsSampler = require('../../dd-trace/src/analytics_sampler') @@ -9,97 +9,99 @@ const HTTP_HEADERS = formats.HTTP_HEADERS const urlFilter = require('../../dd-trace/src/plugins/util/urlfilter') const log = require('../../dd-trace/src/log') const url = require('url') -const { COMPONENT, ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants') +const { CLIENT_PORT_KEY, COMPONENT, ERROR_MESSAGE, ERROR_TYPE, ERROR_STACK } = require('../../dd-trace/src/constants') const HTTP_STATUS_CODE = tags.HTTP_STATUS_CODE const HTTP_REQUEST_HEADERS = tags.HTTP_REQUEST_HEADERS const HTTP_RESPONSE_HEADERS = tags.HTTP_RESPONSE_HEADERS -class HttpClientPlugin extends Plugin { +class HttpClientPlugin extends ClientPlugin { static get id () { return 'http' } - constructor (...args) { - super(...args) - - this.addSub('apm:http:client:request:start', ({ args, http }) => { - const store = storage.getStore() - const options = args.options - const agent = options.agent || options._defaultAgent || http.globalAgent - const protocol = options.protocol || agent.protocol || 'http:' - const hostname = options.hostname || options.host || 'localhost' - const host = options.port ? `${hostname}:${options.port}` : hostname - const path = options.path ? options.path.split(/[?#]/)[0] : '/' - const uri = `${protocol}//${host}${path}` - const allowed = this.config.filter(uri) - - const method = (options.method || 'GET').toUpperCase() - const childOf = store && allowed ? store.span : null - const span = this.tracer.startSpan('http.request', { - childOf, - tags: { - [COMPONENT]: this.constructor.id, - 'span.kind': 'client', - 'service.name': getServiceName(this.tracer, this.config, options), - 'resource.name': method, - 'span.type': 'http', - 'http.method': method, - 'http.url': uri, - 'out.host': hostname - } - }) + addTraceSub (eventName, handler) { + this.addSub(`apm:${this.constructor.id}:client:${this.operation}:${eventName}`, handler) + } - // TODO: Figure out a better way to do this for any span. - if (!allowed) { - span._spanContext._trace.record = false + start ({ args, http }) { + const store = storage.getStore() + const options = args.options + const agent = options.agent || options._defaultAgent || http.globalAgent + const protocol = options.protocol || agent.protocol || 'http:' + const hostname = options.hostname || options.host || 'localhost' + const host = options.port ? `${hostname}:${options.port}` : hostname + const path = options.path ? options.path.split(/[?#]/)[0] : '/' + const uri = `${protocol}//${host}${path}` + const allowed = this.config.filter(uri) + + const method = (options.method || 'GET').toUpperCase() + const childOf = store && allowed ? store.span : null + // TODO delegate to super.startspan + const span = this.startSpan('http.request', { + childOf, + meta: { + [COMPONENT]: this.constructor.id, + 'span.kind': 'client', + 'service.name': getServiceName(this.tracer, this.config, options), + 'resource.name': method, + 'span.type': 'http', + 'http.method': method, + 'http.url': uri, + 'out.host': hostname + }, + metrics: { + [CLIENT_PORT_KEY]: parseInt(options.port) } + }) - if (!(hasAmazonSignature(options) || !this.config.propagationFilter(uri))) { - this.tracer.inject(span, HTTP_HEADERS, options.headers) - } + // TODO: Figure out a better way to do this for any span. + if (!allowed) { + span._spanContext._trace.record = false + } - analyticsSampler.sample(span, this.config.measured) - this.enter(span, store) - }) + if (!(hasAmazonSignature(options) || !this.config.propagationFilter(uri))) { + this.tracer.inject(span, HTTP_HEADERS, options.headers) + } - this.addSub('apm:http:client:request:finish', ({ req, res }) => { - const span = storage.getStore().span - if (res) { - span.setTag(HTTP_STATUS_CODE, res.statusCode) + analyticsSampler.sample(span, this.config.measured) + this.enter(span, store) + } - if (!this.config.validateStatus(res.statusCode)) { - span.setTag('error', 1) - } + finish ({ req, res }) { + const span = storage.getStore().span + if (res) { + span.setTag(HTTP_STATUS_CODE, res.statusCode) - addResponseHeaders(res, span, this.config) + if (!this.config.validateStatus(res.statusCode)) { + span.setTag('error', 1) } - addRequestHeaders(req, span, this.config) + addResponseHeaders(res, span, this.config) + } - this.config.hooks.request(span, req, res) - span.finish() - }) + addRequestHeaders(req, span, this.config) - this.addSub('apm:http:client:request:error', errorHandler) + this.config.hooks.request(span, req, res) + span.finish() } - configure (config) { - return super.configure(normalizeClientConfig(config)) - } -} + error (err) { + const span = storage.getStore().span -function errorHandler (err) { - const span = storage.getStore().span + if (err) { + span.addTags({ + [ERROR_TYPE]: err.name, + [ERROR_MESSAGE]: err.message || err.code, + [ERROR_STACK]: err.stack + }) + } else { + span.setTag('error', 1) + } + } - if (err) { - span.addTags({ - [ERROR_TYPE]: err.name, - [ERROR_MESSAGE]: err.message || err.code, - [ERROR_STACK]: err.stack - }) - } else { - span.setTag('error', 1) + configure (config) { + return super.configure(normalizeClientConfig(config)) } } From cac2cd61926e9a2082bace68d80604fb05655f9e Mon Sep 17 00:00:00 2001 From: Jordi Bertran de Balanda Date: Thu, 11 May 2023 11:26:23 +0200 Subject: [PATCH 2/2] move http2 client to clientPlugin --- packages/datadog-plugin-http2/src/client.js | 97 +++++++++++---------- 1 file changed, 50 insertions(+), 47 deletions(-) diff --git a/packages/datadog-plugin-http2/src/client.js b/packages/datadog-plugin-http2/src/client.js index be9b4730f4a..db33955ac2d 100644 --- a/packages/datadog-plugin-http2/src/client.js +++ b/packages/datadog-plugin-http2/src/client.js @@ -1,7 +1,7 @@ 'use strict' const { storage } = require('../../datadog-core') -const Plugin = require('../../dd-trace/src/plugins/plugin') +const ClientPlugin = require('../../dd-trace/src/plugins/client') const URL = require('url').URL const log = require('../../dd-trace/src/log') @@ -24,7 +24,7 @@ const HTTP2_HEADER_PATH = ':path' const HTTP2_HEADER_STATUS = ':status' const HTTP2_METHOD_GET = 'GET' -class Http2ClientPlugin extends Plugin { +class Http2ClientPlugin extends ClientPlugin { static get id () { return 'http2' } @@ -32,47 +32,6 @@ class Http2ClientPlugin extends Plugin { constructor (...args) { super(...args) - this.addSub('apm:http2:client:request:start', ({ authority, options, headers = {} }) => { - const sessionDetails = extractSessionDetails(authority, options) - const path = headers[HTTP2_HEADER_PATH] || '/' - const pathname = path.split(/[?#]/)[0] - const method = headers[HTTP2_HEADER_METHOD] || HTTP2_METHOD_GET - const uri = `${sessionDetails.protocol}//${sessionDetails.host}:${sessionDetails.port}${pathname}` - const allowed = this.config.filter(uri) - - const store = storage.getStore() - const childOf = store && allowed ? store.span : null - const span = this.tracer.startSpan('http.request', { - childOf, - tags: { - [COMPONENT]: this.constructor.id, - [CLIENT_PORT_KEY]: parseInt(sessionDetails.port), - [SPAN_KIND]: CLIENT, - 'service.name': getServiceName(this.tracer, this.config, sessionDetails), - 'resource.name': method, - 'span.type': 'http', - 'http.method': method, - 'http.url': uri, - 'out.host': sessionDetails.host - } - }) - - // TODO: Figure out a better way to do this for any span. - if (!allowed) { - span._spanContext._trace.record = false - } - - addHeaderTags(span, headers, HTTP_REQUEST_HEADERS, this.config) - - if (!hasAmazonSignature(headers, path)) { - this.tracer.inject(span, HTTP_HEADERS, headers) - } - - analyticsSampler.sample(span, this.config.measured) - - this.enter(span, store) - }) - this.addSub('apm:http2:client:response', (headers) => { const span = storage.getStore().span const status = headers && headers[HTTP2_HEADER_STATUS] @@ -85,14 +44,58 @@ class Http2ClientPlugin extends Plugin { addHeaderTags(span, headers, HTTP_RESPONSE_HEADERS, this.config) }) + } - this.addSub('apm:http2:client:request:finish', () => { - const span = storage.getStore().span + addTraceSub (eventName, handler) { + this.addSub(`apm:${this.constructor.id}:client:${this.operation}:${eventName}`, handler) + } - span.finish() + start ({ authority, options, headers = {} }) { + const sessionDetails = extractSessionDetails(authority, options) + const path = headers[HTTP2_HEADER_PATH] || '/' + const pathname = path.split(/[?#]/)[0] + const method = headers[HTTP2_HEADER_METHOD] || HTTP2_METHOD_GET + const uri = `${sessionDetails.protocol}//${sessionDetails.host}:${sessionDetails.port}${pathname}` + const allowed = this.config.filter(uri) + + const store = storage.getStore() + const childOf = store && allowed ? store.span : null + const span = this.startSpan('http.request', { + childOf, + meta: { + [COMPONENT]: this.constructor.id, + [SPAN_KIND]: CLIENT, + 'service.name': getServiceName(this.tracer, this.config, sessionDetails), + 'resource.name': method, + 'span.type': 'http', + 'http.method': method, + 'http.url': uri, + 'out.host': sessionDetails.host + }, + metrics: { + [CLIENT_PORT_KEY]: parseInt(sessionDetails.port) + } }) - this.addSub('apm:http2:client:request:error', this.addError) + // TODO: Figure out a better way to do this for any span. + if (!allowed) { + span._spanContext._trace.record = false + } + + addHeaderTags(span, headers, HTTP_REQUEST_HEADERS, this.config) + + if (!hasAmazonSignature(headers, path)) { + this.tracer.inject(span, HTTP_HEADERS, headers) + } + + analyticsSampler.sample(span, this.config.measured) + + this.enter(span, store) + } + + finish () { + const span = storage.getStore().span + span.finish() } configure (config) {