opentelemetry-python icon indicating copy to clipboard operation
opentelemetry-python copied to clipboard

Adds support OTEL_SDK_DISABLED environment variable

Open puskardeb opened this issue 1 year ago • 5 comments

Description

Adds support for OTEL_SDK_DISABLED environment variable. If set to "true", the SDK will do no operations.

Fixes #3184

Type of change

Please delete options that are not relevant.

  • [x] New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • [x] Ran locally in console with the help of the following code.
from opentelemetry.sdk.resources import SERVICE_NAME, Resource

from opentelemetry import trace
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import BatchSpanProcessor, ConsoleSpanExporter

from opentelemetry import metrics
from opentelemetry.sdk.metrics import MeterProvider
from opentelemetry.sdk.metrics.export import PeriodicExportingMetricReader, ConsoleMetricExporter

resource = Resource(attributes={
    SERVICE_NAME: "your-service-name"
})

traceProvider = TracerProvider(resource=resource)
processor = BatchSpanProcessor(ConsoleSpanExporter())
traceProvider.add_span_processor(processor)
trace.set_tracer_provider(traceProvider)

reader = PeriodicExportingMetricReader(ConsoleMetricExporter())
meterProvider = MeterProvider(resource=resource, metric_readers=[reader])
metrics.set_meter_provider(meterProvider)

tracer = trace.get_tracer("test tracer")
with tracer.start_as_current_span("test span"):
    print(1)

Output in console:

$ export OTEL_SDK_DISABLED=true
SDK is disabled.
1
$ unset OTEL_SDK_DISABLED
1
{
    "name": "test span",
    "context": {
        "trace_id": "0x19b1ce33587fbe0064b072bde41e1e5d",
        "span_id": "0xaabfc3da341721d4",
        "trace_state": "[]"
    },
    "kind": "SpanKind.INTERNAL",
    "parent_id": null,
    "start_time": "2024-01-24T13:40:37.576284Z",
    "end_time": "2024-01-24T13:40:37.576361Z",
    "status": {
        "status_code": "UNSET"
    },
    "attributes": {},
    "events": [],
    "links": [],
    "resource": {
        "attributes": {
            "service.name": "your-service-name"
        },
        "schema_url": ""
    }
}

Does This PR Require a Contrib Repo Change?

  • [x] No.

Checklist:

  • [x] Followed the style guidelines of this project
  • [ ] Changelogs have been updated
  • [x] Unit tests have been added
  • [ ] Documentation has been updated

puskardeb avatar Jan 24 '24 13:01 puskardeb

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: lzchen / name: Leighton Chen (43fd85b67da250604004c4dead305e4032a43b77)
  • :white_check_mark: login: puskardeb / name: Puskar Deb (bfd45e1261d9947f7bb6e972e0cdee7d0e2f79af, f527bf00c4da877beccc31ff85c317e28bb7b395, 4fadaa1358e73e0bebc104fd0897e36fdff1c33a, a89034dd3e94ab81efe3830834bba0b85444f611, 383a58967edb9797af1fb08111df950d656b3ed1)
  • :white_check_mark: login: ocelotl / name: Diego Hurtado (1582401784aa9b05b576ff0f7dba5e5cefa65c83, b5a3018b45b9207c3a1195b737d87c2bce391c6a)

I don't think this should go in the main SDK code. My understanding is this should be targeted for disabling the auto instrumentation part.

srikanthccv avatar Jan 27 '24 16:01 srikanthccv

I don't think this should go in the main SDK code. My understanding is this should be targeted for disabling the auto instrumentation part.

I went through the environment variable specification and could not find any reference that it should only be for auto instrumentation. Also, had a look at the Javascript library for opentelemetry. Looking at their code for disabling via environment, it did not look as if it is only for auto instrumentation. However I may have understood incorrectly or maybe the implementation of that library is incorrect. Happy to discuss about this.

puskardeb avatar Jan 29 '24 16:01 puskardeb

I looked at the Java support for this, which was the group that proposed this env to the specification. And my understanding is that it should be applied to auto-instrumentation. Let me ask a question in the specification/java slack channel.

srikanthccv avatar Feb 03 '24 10:02 srikanthccv

Hi @srikanthccv, did you get some time to ask around about this?

puskardeb avatar Feb 10 '24 10:02 puskardeb

@srikanthccv

@srikanthccv

It does seem like it's being used in the SDK and not autoinstrumentation for Java so this seems like an appropriate change.

lzchen avatar Mar 07 '24 18:03 lzchen

@lzchen Thanks for the comments. I have updated the PR based on your comments. Also, I think this PR needs a label of "Approve Public API check" which I don't know how to add. If you or someone else can help me out with this, I would be grateful.

puskardeb avatar Mar 08 '24 12:03 puskardeb

Sorry for the lack of response here and yes you are correct.

srikanthccv avatar Mar 12 '24 13:03 srikanthccv

@puskardeb

Please fix the build issues and then we can get this merged!

lzchen avatar Mar 12 '24 17:03 lzchen

@lzchen Regarding the pylint errors, I am having a bit of trouble figuring out why I am getting the error. I have suppressed the error, however if you think there is a better way to resolve this, please feel free to opine. As for the mypy errors, I believe it is not related to my change, I am getting the errors on the main branch when I run tox locally on my setup.

puskardeb avatar Mar 12 '24 20:03 puskardeb

@puskardeb

mypy errors should be fixed with this. Please rebase and we can merge this :)

lzchen avatar Mar 14 '24 18:03 lzchen

@lzchen I have rebased my changes. Thanks :)

puskardeb avatar Mar 14 '24 21:03 puskardeb