feat(sdk-logs)!: move environment variable configuration to sdk-node#6325
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6325 +/- ##
==========================================
- Coverage 95.58% 95.58% -0.01%
==========================================
Files 314 313 -1
Lines 9590 9576 -14
Branches 2221 2210 -11
==========================================
- Hits 9167 9153 -14
Misses 423 423
🚀 New features to boost your workflow:
|
pichlermarc
force-pushed
the
feat/sdk-logs-env-var-config
branch
from
0bd6d2c to
6ce2187
Compare
pichlermarc
force-pushed
the
feat/sdk-logs-env-var-config
branch
from
6ce2187 to
7a06090
Compare
pichlermarc
changed the title
| this._maxExportBatchSize = config?.maxExportBatchSize ?? 512; | ||
| this._maxQueueSize = config?.maxQueueSize ?? 2048; | ||
| this._scheduledDelayMillis = config?.scheduledDelayMillis ?? 5000; | ||
| this._exportTimeoutMillis = config?.exportTimeoutMillis ?? 30000; |
There was a problem hiding this comment.
| logRecordLimits: { | ||
| attributeCountLimit: config.logRecordLimits?.attributeCountLimit ?? 128, | ||
| attributeValueLengthLimit: | ||
| config.logRecordLimits?.attributeValueLengthLimit ?? Infinity, | ||
| }, |
There was a problem hiding this comment.
| } | ||
|
|
||
| // disable all registered SDK components | ||
| context.disable(); |
There was a problem hiding this comment.
just being curious. Was this giving any issue?
There was a problem hiding this comment.
just logspam in the raw test output - when overriding existing API globals, the API logs an error that looks like the test may be failing, see:
opentelemetry-js/api/src/internal/global-utils.ts
Lines 44 to 51 in 4b365bf
It's not solving any issue other than that for now, but it may avoid problems later when we're testing registration here. 🙂
536fd55
|
|
||
| ### :boom: Breaking Changes | ||
|
|
||
| * feat(sdk-logs)!: move environment variable configuration to `@opentelemetry/sdk-node` [#????](https://github.com/open-telemetry/opentelemetry-js/pull/????) @pichlermarc |
There was a problem hiding this comment.
It looks like this landed with the question marks still in place for the issue number.
| 'Long attribute should be truncated to 10 characters' | ||
| ); | ||
|
|
||
| await sdk.shutdown(); |
There was a problem hiding this comment.
if any of the assertions here fail, does it matter that the SDK won't be shutdown?
There was a problem hiding this comment.
In general, it probably can matter (i.e. impact subsequent tests), so this isn't ideal.
…rocessor config PR open-telemetry#6325 accidentally changed a couple envvar names when moving their usage from sdk-logs to sdk-node. The correct names, from the spec, are OTEL_BLRP_SCHEDULE_DELAY and OTEL_BLRP_EXPORT_TIMEOUT. Refs: open-telemetry#6325
4 participants
Which problem is this PR solving?
Moves env var configuration to a sdk-node.
LoggerProvider,BatchLogRecordProcessordo not read environment variables anymore, aligning with sdk-metrics.This is done in preparation for Logs GA, as well as to prepare for declarative config (
createusing file config has to useBatchLogRecordProcessorandLoggerProviderconstructors eventually, by not reading environment variables in the SDK implementation, it makes it easier to adhere to this specification later on, which requires us to ignore env var config when file config is used).To retain env var config for these components, users have to use
@opentelemetry/sdk-node'sNodeSDKinstead.It also adds additional validation to avoid negative numbers.
Closes #6074
Type of change
How Has This Been Tested?