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

[#251] Fix SQL-ID not found sqlId:1 bug #252

Merged
merged 1 commit into from
Dec 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions lib/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const TraceContext = require('./context/trace-context')
const log = require('./utils/logger')
const stringMetaService = require('./context/string-meta-service')
const apiMetaService = require('./context/api-meta-service')
const sqlMetadataService = require('./instrumentation/sql/sql-metadata-service')
const Scheduler = require('./utils/scheduler')
const AgentStatsMonitor = require('./metric/agent-stats-monitor')
const { initializeConfig, getConfig } = require('./config')
Expand All @@ -37,14 +38,13 @@ class Agent {
const agentStartTime = Date.now()
this.agentInfo = this.createAgentInfo(this.config, agentStartTime)

this.initializeDataSender()
this.initializePinpointClient()
const dataSender = this.makeDataSender()
this.dataSender = dataSender
this.initializeDataSender(dataSender)
this.initializePinpointClient(dataSender)

this.traceContext = new TraceContext(this.agentInfo, this.dataSender, this.config)

stringMetaService.init(this.dataSender)
apiMetaService.init(this.dataSender)

this.startSchedule(agentId, agentStartTime)
this.initializeSupportModules()

Expand All @@ -53,17 +53,23 @@ class Agent {
log.warn('[Pinpoint Agent][' + agentId + '] Init Completed')
}

initializeDataSender() {
this.dataSender = dataSenderFactory.create(this.config, this.agentInfo)
this.dataSender.send(this.agentInfo)
makeDataSender() {
return dataSenderFactory.create(this.config, this.agentInfo)
}

initializeDataSender(dataSender) {
dataSender.send(this.agentInfo)
stringMetaService.init(dataSender)
apiMetaService.init(dataSender)
sqlMetadataService.setDataSender(dataSender)
}

initializeSupportModules() {
this.moduleHook = new ModuleHook(this)
}

initializePinpointClient() {
this.pinpointClient = new PinpointClient(this.config, this.agentInfo, this.dataSender)
initializePinpointClient(dataSender) {
this.pinpointClient = new PinpointClient(this.config, this.agentInfo, dataSender)
}

createTraceObject(requestData) {
Expand Down
3 changes: 1 addition & 2 deletions lib/client/data-sender-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@ const GrpcDataSender = require('./grpc-data-sender')
const DataSender = require('./data-sender')

const create = (config, agentInfo) => {
const dataSender = new DataSender(config, new GrpcDataSender(
return new DataSender(config, new GrpcDataSender(
config.collectorIp,
config.collectorTcpPort,
config.collectorStatPort,
config.collectorSpanPort,
agentInfo,
config
))
return dataSender
}

module.exports = {
Expand Down
1 change: 0 additions & 1 deletion lib/client/pinpoint-client.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
'use strict'

const CommandType = require('../constant/commaned-type').CommandType
const log = require('../utils/logger')

class PinpointClient {
constructor(config, agentInfo, dataSender) {
Expand Down
3 changes: 2 additions & 1 deletion lib/context/disable-trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ class DisableTrace {
constructor(traceRoot) {
this.traceRoot = traceRoot
this.spanRecorder = new DisableSpanRecorder()
this.spanEventRecorder = new DisableSpanEventRecorder()
}


traceBlockBegin() {
return new DisableSpanEventRecorder()
return this.spanEventRecorder
}

traceBlockEnd() {}
Expand Down
4 changes: 2 additions & 2 deletions lib/context/span-event-recorder.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const StringMetaService = require('./string-meta-service')
const AsyncId = require('./async-id')
const AnnotationKeyUtils = require('./annotation-key-utils')
const log = require('../utils/logger')
const sqlMetaDataService = require('../instrumentation/sql/sql-metadata-service')
const sqlMetadataService = require('../instrumentation/sql/sql-metadata-service')
const Annotations = require('../instrumentation/context/annotation/annotations')

// https://github.com/pinpoint-apm/pinpoint/blob/master/bootstraps/bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/context/SpanEventRecorder.java
Expand Down Expand Up @@ -157,7 +157,7 @@ class SpanEventRecorder {
return
}

const parsingResult = sqlMetaDataService.cacheSql(sql)
const parsingResult = sqlMetadataService.cacheSql(sql)
this.recordSqlParsingResult(parsingResult, bindString)
return parsingResult
}
Expand Down
13 changes: 7 additions & 6 deletions lib/context/trace/child-trace-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,20 @@ class ChildTraceBuilder {
}

traceBlockBegin(stackId = StackId.default) {
if (this.closed) {
return SpanEventRecorder.nullObject(this.traceRoot)
}

return this.callStack.makeSpanEventRecorder(stackId)
}

traceBlockEnd(spanEventRecorder) {
if (this.closed) {
return
// If ChildTraceBuilder is closed, aync spanEvent was popped in the previous call.
// So, increase the depth of the next spanEvent.
const spanEventBuilder = spanEventRecorder.getSpanEventBuilder()
spanEventBuilder.setDepth(spanEventBuilder.getDepth() + 1)
}

this.endSpanEventBuilder(spanEventRecorder.getSpanEventBuilder())
if (this.closed) {
this.repository.flush()
}
}

endSpanEventBuilder(builder) {
Expand Down
4 changes: 4 additions & 0 deletions lib/context/trace/disable-child-trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ class DisableChildTrace {

traceBlockEnd() {}

getSpanRecorder() {
return this.spanRecorder
}

getTraceRoot() {
return this.traceRoot
}
Expand Down
4 changes: 4 additions & 0 deletions lib/context/trace/span-event-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ class SpanEventBuilder {
return this
}

getDepth() {
return this.depth
}

getStartTime() {
return this.startTime
}
Expand Down
4 changes: 2 additions & 2 deletions lib/context/trace/span-event-recorder2.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const Annotations = require('../../instrumentation/context/annotation/annotation
const SpanEventBuilder = require('./span-event-builder')
const log = require('../../utils/logger')
const AnnotationKeyUtils = require('../annotation-key-utils')
const sqlMetaDataService = require('../../instrumentation/sql/sql-metadata-service')
const sqlMetadataService = require('../../instrumentation/sql/sql-metadata-service')
const stringMetaService = require('../string-meta-service')
const AsyncId = require('../async-id')

Expand Down Expand Up @@ -127,7 +127,7 @@ class SpanEventRecorder {
return
}

const parsingResult = sqlMetaDataService.cacheSql(sql)
const parsingResult = sqlMetadataService.cacheSql(sql)
this.recordSqlParsingResult(parsingResult, bindString)
return parsingResult
}
Expand Down
8 changes: 8 additions & 0 deletions lib/instrumentation/instrument-arrow-function.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
const shimmer = require('@pinpoint-apm/shimmer')
const HookFunctionArguments = require('./hook-function-arguments')
const localStorage = require('./context/local-storage')
// const log = require('../utils/logger')

// ref: SpanEventSimpleAroundInterceptorForPlugin.java
class InstrumentArrowFunction {
Expand Down Expand Up @@ -65,6 +66,13 @@ class InstrumentArrowFunction {
return original.apply(this, arguments)
}

// if (log.isInfo()) {
// if (typeof method !== 'symbol') {
// log.info(`target: ${JSON.stringify(target)}, method: ${method}, original: ${original} InstrumentArrowFunction.addChildTraceInterceptor`)
// } else {
// log.info(`target: ${JSON.stringify(target)}, symbol method, original: ${original} InstrumentArrowFunction.addChildTraceInterceptor`)
// }
// }
const childTraceBuilder = traceContext.continueAsyncContextTraceObject(trace.getTraceRoot(), asyncId.nextLocalAsyncId2())
return localStorage.run(childTraceBuilder, () => {
const result = original.apply(this, arguments)
Expand Down
16 changes: 5 additions & 11 deletions lib/instrumentation/sql/sql-metadata-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,13 @@ const SimpleCache = require('../../utils/simple-cache')
const SqlParser = require('./sql-parser')
const ParsingResult = require('./parsing-result')

class SqlMetaDataService {
class SqlMetadataService {
constructor() {
this.cache = new SimpleCache(1024)
}

set dataSender(dataSender) {
this._dataSender = dataSender
}

get dataSender() {
return this._dataSender
setDataSender(dataSender) {
this.dataSender = dataSender
}

cacheSql(sql) {
Expand All @@ -38,10 +34,8 @@ class SqlMetaDataService {
}

send(sqlMetaData) {
if (this.dataSender && typeof this.dataSender.send === 'function') {
this.dataSender.send(sqlMetaData)
}
this.dataSender?.send?.(sqlMetaData)
}
}

module.exports = new SqlMetaDataService()
module.exports = new SqlMetadataService()
5 changes: 4 additions & 1 deletion test/client/data-sender.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

const test = require('tape')

require('../support/agent-singleton-mock')
const agent = require('../support/agent-singleton-mock')
const { fixture } = require('../test-helper')
const dataSenderMock = require('../support/data-sender-mock')
const dataSender = dataSenderMock()
Expand All @@ -18,6 +18,7 @@ const defaultPredefinedMethodDescriptorRegistry = require('../../lib/constant/de

test('Should send agent info', function (t) {
t.plan(1)
agent.bindHttp()

dataSender.send(agentInfo)

Expand All @@ -26,6 +27,7 @@ test('Should send agent info', function (t) {

test('Should send api meta info', function (t) {
t.plan(1)
agent.bindHttp()

const methodDescriptor = defaultPredefinedMethodDescriptorRegistry.nodeServerMethodDescriptor
const apiMetaInfo = ApiMetaInfo.create(methodDescriptor)
Expand All @@ -36,6 +38,7 @@ test('Should send api meta info', function (t) {

test('Should send string meta info', function (t) {
t.plan(1)
agent.bindHttp()

const stringMetaInfo = StringMetaInfo.create('1', 'test string')
dataSender.send(stringMetaInfo)
Expand Down
10 changes: 5 additions & 5 deletions test/client/grpc-data-sender.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ const AsyncSpanChunkBuilder = require('../../lib/context/trace/async-span-chunk-
const SpanRepository = require('../../lib/context/trace/span-repository')
const ChildTraceBuilder = require('../../lib/context/trace/child-trace-builder')
const serviceType = require('../../lib/context/service-type')
const makeMockDataSender = require('../fixtures/mock-data-sender')
const SpanChunkBuilder = require('../../lib/context/span-chunk-builder')
const Trace = require('../../lib/context/trace/trace2')
const defaultPredefinedMethodDescriptorRegistry = require('../../lib/constant/default-predefined-method-descriptor-registry')
const dataSenderMock = require('../support/data-sender-mock')

let sendSpanMethodOnDataCallback
function sendSpan(call) {
Expand Down Expand Up @@ -70,7 +70,7 @@ test('Should send span', function (t) {
server.bindAsync('localhost:0', grpc.ServerCredentials.createInsecure(), (error, port) => {
const grpcDataSender = beforeSpecificOne(port, DataSource)
const traceRoot = new RemoteTraceRootBuilder(agent.agentInfo, '5').build()
dataSender = makeMockDataSender(agent.config, grpcDataSender)
dataSender = dataSenderMock(agent.config, grpcDataSender)
const spanBuilder = new SpanBuilder(traceRoot)
spanBuilder.setServiceType(1400)
spanBuilder.setEndPoint('localhost:3000')
Expand Down Expand Up @@ -167,7 +167,7 @@ test('sendSpanChunk redis.SET.end', function (t) {
const grpcDataSender = beforeSpecificOne(port, DataSource)
const traceRoot = new RemoteTraceRootBuilder(agent.agentInfo, '5').build()
const asyncId = AsyncId.make()
dataSender = makeMockDataSender(agent.config, grpcDataSender)
dataSender = dataSenderMock(agent.config, grpcDataSender)
const spanChunkBuilder = new AsyncSpanChunkBuilder(traceRoot, asyncId)
const repository = new SpanRepository(spanChunkBuilder, dataSender, agent.agentInfo)
const childTraceBuilder = new ChildTraceBuilder(traceRoot, repository, asyncId)
Expand Down Expand Up @@ -245,7 +245,7 @@ test('sendSpanChunk redis.GET.end', (t) => {
const grpcDataSender = beforeSpecificOne(port, DataSource)
const traceRoot = new RemoteTraceRootBuilder(agent.agentInfo, '5').build()
const asyncId = AsyncId.make()
dataSender = makeMockDataSender(agent.config, grpcDataSender)
dataSender = dataSenderMock(agent.config, grpcDataSender)
const spanChunkBuilder = new AsyncSpanChunkBuilder(traceRoot, asyncId)
const repository = new SpanRepository(spanChunkBuilder, dataSender, agent.agentInfo)
const childTraceBuilder = new ChildTraceBuilder(traceRoot, repository, asyncId)
Expand Down Expand Up @@ -315,7 +315,7 @@ test('sendSpan', (t) => {
server.bindAsync('localhost:0', grpc.ServerCredentials.createInsecure(), (error, port) => {
const grpcDataSender = beforeSpecificOne(port, DataSource)
const traceRoot = new RemoteTraceRootBuilder(agent.agentInfo, '5').build()
dataSender = makeMockDataSender(agent.config, grpcDataSender)
dataSender = dataSenderMock(agent.config, grpcDataSender)
const spanBuilder = new SpanBuilder(traceRoot)
const spanChunkBuilder = new SpanChunkBuilder(traceRoot)
const repository = new SpanRepository(spanChunkBuilder, dataSender, agent.agentInfo)
Expand Down
Loading
Loading