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

chore: clean up http proto exporter instantiation #75

Merged
merged 3 commits into from
Oct 19, 2023
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
12 changes: 2 additions & 10 deletions create-log-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,7 @@ function createExporter (exporterOptions) {
return new ConsoleLogRecordExporter()
}

const {
OTLPLogsExporter,
OTLPLogExporter
} = require('@opentelemetry/exporter-logs-otlp-proto')
const { OTLPLogExporter } = require('@opentelemetry/exporter-logs-otlp-proto')

if (typeof OTLPLogExporter === 'function') {
return new OTLPLogExporter(exporterOptions?.protobufExporterOptions)
}

// TODO: remove this once https:/open-telemetry/opentelemetry-js/issues/3812#issuecomment-1713830883 is resolved
return new OTLPLogsExporter(exporterOptions?.protobufExporterOptions)
return new OTLPLogExporter(exporterOptions?.protobufExporterOptions)
}
19 changes: 0 additions & 19 deletions otlp-logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,6 @@ const {

const DEFAULT_MESSAGE_KEY = 'msg'

// TODO: BatchLogRecordProcessor should be configurable with https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#batch-logrecord-processor
// which implements this spec https://opentelemetry.io/docs/specs/otel/logs/sdk/#batching-processor
// and is implemented here https:/open-telemetry/opentelemetry-js/blob/48fb15862e801b742059a3e39dbcc8ef4c10b2e2/experimental/packages/sdk-logs/src/export/BatchLogRecordProcessorBase.ts#L47C1-L47C1
//
//
// TODO: document the ability for user to provide one's own LogRecorProcesor https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecordprocessor
//
// TODO: document thow the user can create a MultiLogProcessor if they need to use multiple exporters https:/open-telemetry/opentelemetry-js/blob/main/experimental/packages/sdk-logs/src/MultiLogRecordProcessor.ts
// TODO: use MultiLogRecordProcessor to support multiple exporters as an implementation detail as the sdk does not export that
//
//
// All env vars are defined here: https:/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-core/src/utils/environment.ts#L135
// We might want to read this env var https:/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-core/src/utils/environment.ts#L138
// order: OTEL_EXPORTER_OTLP_LOGS_PROTOCOL ?? OTEL_EXPORTER_OTLP_PROTOCOL ?? options.exporterProtocol
//
//
// TODO: add this chunk to REAMDE "Settings configured programmatically take precedence over environment variables. Per-signal environment variables take precedence over non-per-signal environment variables."
//

/**
* @typedef {Object} Options
* @property {string} loggerName
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"@opentelemetry/exporter-logs-otlp-http": "^0.44.0",
"@opentelemetry/exporter-logs-otlp-proto": "^0.44.0",
"@opentelemetry/resources": "^1.17.0",
"@opentelemetry/sdk-logs": "^0.43.0",
"@opentelemetry/sdk-logs": "^0.44.0",
"pino-abstract-transport": "^1.1.0"
},
"types": "./types/pino-opentelemetry-transport.d.ts",
Expand Down
2 changes: 0 additions & 2 deletions test/otlp-logger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ test('otlp logger logs a record in log exporter and maps all log levels correctl
match(records[0].resource, {
_attributes: {
'service.name': 'test-service',
'telemetry.sdk.language': 'nodejs',
'telemetry.sdk.name': 'opentelemetry',
'service.version': '1.0.0',
foo: 'bar'
}
Expand Down
41 changes: 22 additions & 19 deletions test/pino-opentelemetry-transport.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,24 +97,20 @@ test('translate Pino log format to Open Telemetry data format for each log level
logger.error('test error')
logger.fatal('test fatal')

const resource = {
attributes: [
{
key: 'service.name',
value: {
stringValue: 'test-service'
}
},
{
key: 'telemetry.sdk.language',
value: { stringValue: 'nodejs' }
},
{
key: 'telemetry.sdk.name',
value: { stringValue: 'opentelemetry' }
const expectedResourceAttributes = [
{
key: 'service.name',
value: {
stringValue: 'test-service'
}
]
}
},
{
key: 'service.version',
value: {
stringValue: 'test-service-version'
}
}
]

const scope = {
name: 'test-logger-name',
Expand Down Expand Up @@ -181,7 +177,8 @@ test('translate Pino log format to Open Telemetry data format for each log level
})
.on('err', line => console.error(line))

for await (const _ of setInterval(0)) { //eslint-disable-line
// eslint-disable-next-line
for await (const _ of setInterval(0)) {
if (logRecordReceivedOnCollectorCount >= expectedLines.length) {
break
}
Expand Down Expand Up @@ -211,7 +208,13 @@ test('translate Pino log format to Open Telemetry data format for each log level
same(lines.length, expectedLines.length, 'correct number of lines')

lines.forEach(line => {
hasStrict(JSON.parse(line).resourceLogs?.[0]?.resource, resource)
const foundAttributes = JSON.parse(
line
).resourceLogs?.[0]?.resource.attributes.filter(
attribute =>
attribute.key === 'service.name' || attribute.key === 'service.version'
)
hasStrict(foundAttributes, expectedResourceAttributes)
})

lines.forEach(line => {
Expand Down