Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
1.98k stars 1.15k forks source link

Is `enableTraceBasedSamplingForLogs` ready to be used? #30184

Open marcus13371337 opened 1 week ago

marcus13371337 commented 1 week ago

Is your feature request related to a problem? Please describe. Currently, we are using this package to track traces and logs. We want 100% of the logs to be reported but we are fine with a sampling-rate for traces.

Describe the solution you'd like It seems as this repository already contain this functionality, but it's unclear if it's ready to be used or not. That by providing the following flag to options enableTraceBasedSamplingForLogs

Additional context It seems as the feature is already implemented here: https://github.com/Azure/azure-sdk-for-js/blob/e1702eaaa81d388012cc25b3bfa4a8d3018f7467/sdk/monitor/monitor-opentelemetry/src/logs/batchLogRecordProcessor.ts#L24C23-L24C54

But in the readme I can find the following:


|Property|Description|Default|
| ------------------------------- |------------------------------------------------------------------------------------------------------------|-------|
| azureMonitorExporterOptions     | Azure Monitor OpenTelemetry Exporter Configuration. [More info here](https://github.com/Azure/azure-sdk-for-js/tree/main/sdk/monitor/monitor-opentelemetry-exporter) | | | |
| samplingRatio              | Sampling ratio must take a value in the range [0,1], 1 meaning all data will sampled and 0 all Tracing data will be sampled out. | 1|
| instrumentationOptions| Allow configuration of OpenTelemetry Instrumentations. |  {"http": { enabled: true },"azureSdk": { enabled: false },"mongoDb": { enabled: false },"mySql": { enabled: false },"postgreSql": { enabled: false },"redis": { enabled: false },"bunyan": { enabled: false }, "winston": { enabled: false } }|
| browserSdkLoaderOptions| Allow configuration of Web Instrumentations. | { enabled: false, connectionString: "" } |
| resource       | Opentelemetry Resource. [More info here](https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-resources) ||
| samplingRatio              | Sampling ratio must take a value in the range [0,1], 1 meaning all data will sampled and 0 all Tracing data will be sampled out. |1|
| enableLiveMetrics          | Enable/Disable Live Metrics. |true|
| enableStandardMetrics      | Enable/Disable Standard Metrics. |true|
| logRecordProcessors        | Array of log record processors to register to the global logger provider. ||
| spanProcessors             | Array of span processors to register to the global tracer provider. ||
<!--- TODO: Enable when feature is released | enableTraceBasedSamplingForLogs      | Enable log sampling based on trace. |true|-->

So basically, my question is, is the enableTraceBasedSamplingForLogs-flag ready to be used in production?

hectorhdzg commented 1 week ago

@marcus13371337 the option is ready to be used, I will make sure is documented, we missed that when we released the package, according to what you want you should turn it off as is enabled by default.

marcus13371337 commented 1 week ago

Cool thanks for the clarification!