Skip to content

Commit

Permalink
[#251] Fix SQL-ID not found sqlId:1 bug
Browse files Browse the repository at this point in the history
* GrpcDataSender fixtures close resources
* Functional Test port number
  • Loading branch information
feelform committed Dec 24, 2024
1 parent 9adf4d7 commit f594c36
Show file tree
Hide file tree
Showing 15 changed files with 176 additions and 159 deletions.
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
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
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
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
35 changes: 26 additions & 9 deletions test/client/mock-grpc-data-sender.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,27 @@
'use strict'
const GrpcDataSender = require('../../lib/client/grpc-data-sender')

class MockGrpcStream {
constructor(stream) {
this.grpcStream = stream
}

write(data) {
this.grpcStream.write(data)
}

end() {
}
}

class MockGrpcDataSender extends GrpcDataSender {
initializeClients() {
let self = this
this.agentClient = {
requestAgentInfo: function (pAgentInfo) {
self.actualAgentInfo = pAgentInfo
},
close: function () {
}
}

Expand All @@ -28,12 +43,14 @@ class MockGrpcDataSender extends GrpcDataSender {
},
requestSqlUidMetaData: function (pSqlUidMetaData) {
self.actualSqlUidMetaData = pSqlUidMetaData
},
close: function () {
}
}
this.actualSpans = []
}

get actualSpan () {
get actualSpan() {
return this.actualSpans[this.actualSpans.length - 1]
}

Expand All @@ -42,19 +59,19 @@ class MockGrpcDataSender extends GrpcDataSender {

initializeSpanStream() {
let self = this
this.spanStream = {
this.spanStream = new MockGrpcStream({
write: function (span) {
self.actualSpans.push(span)
},
end: function () {

}
}
})
}

initializeProfilerClients() {
let self = this
this.commandStream = {
this.commandStream = new MockGrpcStream({
write: function (pmessage) {
self.actualPCmdMessage = pmessage
},
Expand All @@ -64,12 +81,12 @@ class MockGrpcDataSender extends GrpcDataSender {
on: function () {

}
}
})
}

initializeStatStream() {
let self = this
this.statStream = {
this.statStream = new MockGrpcStream({
write: function (pmessage) {
self.actualPStatMessage = pmessage
},
Expand All @@ -79,12 +96,12 @@ class MockGrpcDataSender extends GrpcDataSender {
on: function () {

}
}
})
}

initializePingStream() {
let self = this
this.pingStream = {
this.pingStream = new MockGrpcStream({
write: function (pmessage) {
self.actualPingMessage = pmessage
},
Expand All @@ -94,7 +111,7 @@ class MockGrpcDataSender extends GrpcDataSender {
on: function () {

}
}
})
}

initializeAgentInfoScheduler() {
Expand Down
Loading

0 comments on commit f594c36

Please sign in to comment.