[REQUIRED FIRST] Fix lint by making pkg tests folders namespace pkgs

[NathanielRN]

Description

Solves the issue with the lint tests by disabling the few remaining lint errors. We already ignore the import-error case for many tests/ directories so this follows nicely. Also needed to update to pylint==2.4.4 to solve some test issues.

It is curious that this didn't fail in the main repo... I was thinking maybe because opentelemetry-test package is closer to {toxinidir} root? This was a helpful comment:

https://github.com/open-telemetry/opentelemetry-python/blob/master/docs/examples/opentracing/rediscache.py#L10-L14

Fixes #127

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The tests work now!

Checklist:

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

[toumorokoshi]

Would you mind posting the error that you're seeing? I have a feeling this is caused by the lack of a namespace declaration for the tests module:

https://github.com/open-telemetry/opentelemetry-python/blob/master/opentelemetry-api/tests/__init__.py#L22.

Which is waiting on a pylint fix: pylint-dev/pylint#3609.

[toumorokoshi]

Considering excluding lint rules is manual and also hard to inform a user who encounters these issues, I'd vote for re-adding the workaround.

[NathanielRN]

@toumorokoshi This is the error I'm seeing:

************* Module tests.test_pymemcache
instrumentation/opentelemetry-instrumentation-pymemcache/tests/test_pymemcache.py:30:0: E0401: Unable to import 'tests.utils' (import-error)
************* Module tests.test_tasks
instrumentation/opentelemetry-instrumentation-celery/tests/test_tasks.py:22:0: E0401: Unable to import 'tests.celery_test_tasks' (import-error)
************* Module tests.test_client_interceptor
instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py:25:0: E0611: No name 'protobuf' in module 'tests' (no-name-in-module)
instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py:25:0: E0401: Unable to import 'tests.protobuf' (import-error)
instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py:27:0: E0401: Unable to import 'tests._client' (import-error)
instrumentation/opentelemetry-instrumentation-grpc/tests/test_client_interceptor.py:33:0: E0401: Unable to import 'tests._server' (import-error)
************* Module tests._client
instrumentation/opentelemetry-instrumentation-grpc/tests/_client.py:15:0: E0401: Unable to import 'tests.protobuf.test_server_pb2' (import-error)
************* Module tests._server
instrumentation/opentelemetry-instrumentation-grpc/tests/_server.py:19:0: E0401: Unable to import 'tests.protobuf' (import-error)
instrumentation/opentelemetry-instrumentation-grpc/tests/_server.py:25:4: C0103: Method name "SimpleMethod" doesn't conform to snake_case naming style ('(([a-z_][a-z0-9_]{2,})|(_[a-z0-9_]*)|(__[a-z][a-z0-9_]+__))$' pattern) (invalid-name)
instrumentation/opentelemetry-instrumentation-grpc/tests/_server.py:25:4: R0201: Method could be a function (no-self-use)
instrumentation/opentelemetry-instrumentation-grpc/tests/_server.py:34:4: C0103: Method name "ClientStreamingMethod" doesn't conform to snake_case naming style ('(([a-z_][a-z0-9_]{2,})|(_[a-z0-9_]*)|(__[a-z][a-z0-9_]+__))$' pattern) (invalid-name)
instrumentation/opentelemetry-instrumentation-grpc/tests/_server.py:34:4: R0201: Method could be a function (no-self-use)
instrumentation/opentelemetry-instrumentation-grpc/tests/_server.py:44:4: C0103: Method name "ServerStreamingMethod" doesn't conform to snake_case naming style ('(([a-z_][a-z0-9_]{2,})|(_[a-z0-9_]*)|(__[a-z][a-z0-9_]+__))$' pattern) (invalid-name)
instrumentation/opentelemetry-instrumentation-grpc/tests/_server.py:44:4: R0201: Method could be a function (no-self-use)
instrumentation/opentelemetry-instrumentation-grpc/tests/_server.py:63:4: C0103: Method name "BidirectionalStreamingMethod" doesn't conform to snake_case naming style ('(([a-z_][a-z0-9_]{2,})|(_[a-z0-9_]*)|(__[a-z][a-z0-9_]+__))$' pattern) (invalid-name)
instrumentation/opentelemetry-instrumentation-grpc/tests/_server.py:63:4: R0201: Method could be a function (no-self-use)
************* Module tests.test_falcon
instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py:24:0: E0401: Unable to import 'tests.app' (import-error)
************* Module tests.test_instrumentation
instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py:30:0: E0401: Unable to import 'tests.tornado_test_app' (import-error)

Wow I think you're definitely right. I didn't notice that comment 😓

I agree adding the disables is not very intuitive, which workaround do you mean? Do you mean the pkg_resources.declare_namespace(__name__) and to which module do you think it should be added to?

[NathanielRN]

@toumorokoshi Please let me know if I misunderstood the needed change, but when I made each of these packages that had errors include this workaround, it still seemed to fail 😕

I'm suspicious that because there are also "naming" lint errors, there's something else I'm missing. Previous what fixed it is a version bump for pylint so may the right answer is to just merge this and wait for the next version to fix all of this.

Here's the run for my code: https://github.com/NathanielRN/opentelemetry-python-contrib-1/runs/1359261481?check_suite_focus=true

And here's the diff where I added the workaround to all those modules: master...NathanielRN:test-namespace-module

[NathanielRN]

No idea why there's a sdkextension test pending here. Will update against master branch.

[toumorokoshi]

@toumorokoshi Please let me know if I misunderstood the needed change, but when I made each of these packages that had errors include this workaround, it still seemed to fail

It actually needs to be the first package imported. This is a bug in pylint where the python 3.4+ way of declaring a namespace (not including an init.py file) is not recognized properly. Looking at the first few lines of your github actions job:

pylint instrumentation/opentelemetry-instrumentation-wsgi/setup.py 
instrumentation/opentelemetry-instrumentation-wsgi/tests
 instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry
 instrumentation/opentelemetry-instrumentation-dbapi/setup.py
 instrumentation/opentelemetry-instrumentation-dbapi/tests
 instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry
 instrumentation/opentelemetry-instrumentation-asgi/setup.py 

If the first package doesn't declare the package as a namespace, then it'll be considered a module, and thus won't attempt to load any more. See the packaging documentation for more information

The first "tests" module is opentelemetry-instrumentation-wsgi. I put the namespace hack in there and it ran fine.

Can you give that a shot and see if it works? I see your point about getting incremental changes in, but I think in this case we'd just reverse this PR as is once we get namespaces working properly.

[NathanielRN]

Can you give that a shot and see if it works? I see your point about getting incremental changes in, but I think in this case we'd just reverse this PR as is once we get namespaces working properly.

You're awesome @toumorokoshi ! It works! Updated the PR :)

[gnagel]

Very excited to see this fix :-)