Apache plugin does not seem to be working after update

[raimov-uptime]

Hello.

I had apache plugin installed and working correctly but after an update it does not work. When I debug the plugin I get:
python.d ERROR: apache: apache: Url: http://localhost/server-status?auto. Error: 'module' object has no attribute 'Retry'
That url is working if I try it manually. I used kickstart.sh for installation and it runs on amazon ec2.

[ilyam8]

Hi @raimov-uptime

It seems your system has very old urllib3 package. Please check version.

Can you update it?

[ilyam8]

@raimov-uptime
please check the fix
#5109

[raimov-uptime]

@ilyam8 Upgrading urllib3 package worked. Thanks (also for the fix)

[ilyam8]

@raimov-uptime what version was the package?

[ilyam8]

https://pypi.org/project/urllib3/1.12/#history

Current is 1.24.1 (November 2018)

I checked 1.12 (September 2015) and it has Retry object

[raimov-uptime]

It was version 1.8.2.

[ilyam8]

Oh man, 1.8.2 is April 2014 😄

Maybe it's a good idea to not merge the fix and force people to upgrade.

@Ferroin what do you think?

[ilyam8]

@cakrit same question

[cakrit]

Force upgrade is fine, but I think that the installer/updater should be checking for the required version.

[Ferroin]

I'm with @cakrit on this one, it makes no sense to support such ancient versions, but the installer should check versions for everything we depend on and issue appropriate errors or warnings if the version requirement isn't met.

[paulfantom]

We are vendoring urllib3 package so there is no need to do such checks: https://github.com/netdata/netdata/tree/master/collectors/python.d.plugin/python_modules/urllib3

[ilyam8]

netdata/collectors/python.d.plugin/python.d.plugin.in

Line 39 in fd68066

sys.path.append(PYTHON_MODULES_DIR)

append

[paulfantom]

But why?! We are vendoring libraries which we need, so why we even use system ones?

[ilyam8]

I am not sure the urllib3 library, we shipping with netdata, will work on ancient systems.

Actually it will. Netdata's urllib3 version is 1.21

I see from the change log they drop support for py2.6 only in 1.24

[Ferroin]

The problem is that we kind of need to supplement the system libraries. There are a lot of our python.d modules that depend on specific third-party Python libraries which need to be installed on the system, and those need to find the versions of modules they expect to.

So, we either need to support using the system libraries, or we need to vendor everything. This goes double for urllib3, because it's pretty widely used by other Python modules. We theoretically could namespace things so there is no collision issue, but that ends up resulting in us using a lot more memory in a some cases because it means we have multiple versions of the code in memory.

[Ferroin]

Also, when we move to actual packaging, we should be moving away from vendoring things like this and switch to using proper package dependencies (because we'll be able to properly leverage the package manager at that point).

[ilyam8]

@paulfantom is talking about import order, we can do insert instead of append.

[ilyam8]

Rename urllib3 to something will work too 😄

[Ferroin]

@paulfantom is talking about import order, we can do insert instead of append.

But that still runs the risk of breaking third-party libraries that our modules depend on which in turn depend on urllib3. Actually breaking anything that way is probably not very likely, but it's still worth considering.

Rename urllib3 to something will work too

Yes, but that will run the risk of increasing memory usage. That, and ISTR that Python does some form of caching of imports, which might complicate this (though i can't recall if it's by computed path to the import, or by the requested module name).

[paulfantom]

I completely agree with those:

we can do insert instead of append.

This way we would ensure that we are using what we are shipping.

we should be moving away from vendoring things like this and switch to using proper package dependencies

It should be done in the beginning, but sadly it couldn't be 😞

I kinda hope we won't merge go.d.plugin into netdata/netdata repository as this would pave a path to move python module to separate repo where we could maintain library dependencies with pip.

[Ferroin]

we can do insert instead of append.

This way we would ensure that we are using what we are shipping.

As mentioned above, it still runs the risk of breaking other libraries we depend on that we don't vendor (and shouldn't, if we vendored everything we'd be spending more time keeping things up to date and punting bugs upstream that we'd get no new development done) . At minimum, we would need to namespace our usage of urllib3 and reset the import paths before we load the modules.

[ilyam8]

urllib3 is the only problem package and i don't think we will vendor something else in the future

1. urllib3 => nurllib3

try:
    import nurllib3 as urllib3
except ImportError:
    import urllib3

3. profit

[ilyam8]

As mentioned above, it still runs the risk of breaking other libraries we depend on that we don't vendor

please give an example

[Ferroin]

As mentioned above, it still runs the risk of breaking other libraries we depend on that we don't vendor

please give an example

Same exact type of thing happening here, just for a third-party library instead of us. If any of the third-party libraries a module depends on depend on a newer version of urllib3 than what we're shipping, changing our sys.path manipulations from append to insert will break them (because they'll get our copy of urllib3 instead of the system copy). The problem arises from the fact that we can't know for any arbitrary system if whatever versions of those third-party libraries that may be installed on that system will break from this or not.

[paulfantom]

@Ferroin what do you propose?

[Ferroin]

I think the rename approach is fine, but we should only be searching in our module directory for our own imports when stuff is vendored. This issue pretty much demonstrates that we want to explicitly avoid using the system libraries, so it looks like we should be using a custom sys.path only for our imports, and use the default for everything else. I'm not sure what the best way to go about that this is though.

[ilyam8]

I think the rename approach is fine, but we should only be searching in our module directory for our own imports when stuff is vendored.

No. Look at debian/ubuntu netdata package. They don't include our pyyaml and urllib3. The thing is order - our, then system.

[ilyam8]

Rename or move vendored stuff to a separate package will do
example:

try:
    from our import urllib3
except ImportError:
  ...
or
try:
    import nurllib3 as urllib3
except ImportError:
...

[Ferroin]

I like the first approach a bit better, but we still need to make sure we're not polluting things for third-party libraries our modules are using.

I'd propose instead:

try:
    from netdata import urllib3 as nd_urllib3
except ImportError:
   ...

Using netdata for the package name makes it a bit clearer where the module is comming from, and having the prefix means that we don't have to worry about issues like this one happening for any libraries our collector modules depend on.

[ilyam8]

urllib3 as nd_urllib3

not needed if

from netdata import urllib3

Actually using system package is OK. The OP problem that we don't check version.