Fix the auto instrumentation command

[ocelotl]

Fixes #566

[mauriciovasquezbernal]

I tested and it works for me. I have a blocking comment on the PYTHONPATH logic.

Also, please don't forget to update the documentation

opentelemetry-python/opentelemetry-auto-instrumentation/src/opentelemetry/auto_instrumentation/__init__.py

Line 23 in 08b100f

opentelemetry-auto-instrumentation program.py

.

[mauriciovasquezbernal]

I think it's good to go. Just one question about the cwd path being added.

[c24t]

This looks like it works as intended, but I don't understand the intended use of sitecustomize outside of run.

This looks good to unblock the other instrumentation PRs, but I think we need better documentation for (or I need a better understanding of) when this package should patch others.

[c24t]

If I understand this, the goal is to load all available instrumentations if the auto_instrumentation package is on the path, which should only be true if the user called auto_instrumentation/auto_instrumentation.py:run?

Why did you decide to use sitecustomize to do this instead of leaving this in run? Loading the instrumentations as an import effect seems dangerous, but I don't know if this is conventional for sitecustomize.

[toumorokoshi]

I did a little research here. Looks like this is how ddtrace does it too: https://github.com/DataDog/dd-trace-py/blob/master/ddtrace/bootstrap/sitecustomize.py

The main issue that this PR is fixing is the loading + activating of the instrumentations before anything in the downstream script is invoked. In commands like flask_run, you have no control over the actual code, so you can't programatically insert the instrumentation activation into the script itself.

in this scenario the "run" entry point is never invoked by the script that is being execl'd, so auto_instrumentation.py:run will never actually execute in the new python process.

There's very few places that you can run arbitary python code before the script itself starts to run. One of those that I'm aware of is sitecustomize. I'm having trouble thinking of any other place to inject startup code that is guaranteed to run before the script that you're trying to invoke.

[ocelotl]

Yes, exactly. As @toumorokoshi says, this has been changed to support the frameworks or libraries that use launcher executables (thus requiring execl) and are executed in a new Python process.

[majorgreys]

This change is similar to how ddtrace-run works to add the directory where thesitecustomize.py @toumorokoshi just linked is located to the PYTHONPATH before calling execl:

https://github.com/DataDog/dd-trace-py/blob/master/ddtrace/commands/ddtrace_run.py

I tried this PR out with the example auto-instrumentation but using uwsgi to load the app and it worked as expected:

$ opentelemetry-auto-instrumentation uwsgi --http :8082 -w server_uninstrumented:app
*** Starting uWSGI 2.0.18 (64bit) on [Thu Apr 30 13:59:02 2020] ***
....
spawned uWSGI worker 1 (and the only) (pid: 54166, cores: 1)
testing
{
    "name": "server_request",
    "context": {
        "trace_id": "0x3416621a548ee3b57495a4ecc3bc9502",
        "span_id": "0xfc0221256e37be82",
        "trace_state": "{}"
    },
    "kind": "SpanKind.SERVER",
    "parent_id": "0xdee7b970d0693b48",
    "start_time": "2020-04-30T17:59:32.630551Z",
    "end_time": "2020-04-30T17:59:32.632212Z",
    "status": {
        "canonical_code": "OK"
    },
    "attributes": {
        "component": "http",
        "http.method": "GET",
        "http.server_name": "tbutt.local",
        "http.scheme": "http",
        "host.port": 8082,
        "http.host": "localhost:8082",
        "http.target": "/server_request?param=testing",
        "net.peer.ip": "127.0.0.1",
        "net.peer.port": "40658",
        "http.flavor": "1.1",
        "http.route": "/server_request",
        "http.status_text": "OK",
        "http.status_code": 200
    },
    "events": [],
    "links": []
}