community.general icon indicating copy to clipboard operation
community.general copied to clipboard

Support for opentelemetry-api 1.13.0

Open cpiment opened this issue 3 years ago • 6 comments

Summary

When trying to use the opentelemetry callback with opentelemetry 1.13 this warning arises:

[WARNING]: Skipping callback 'community.general.opentelemetry', unable to load due to: The `opentelemetry-api`, `opentelemetry-exporter-otlp` or `opentelemetry-sdk` must be installed to use this plugin

I have debugged the error and it raises when it reaches this line: https://github.com/ansible-collections/community.general/blob/main/plugins/callback/opentelemetry.py#L113

opentelemetry.util._time has been removed in this commit from OpenTelemetry: https://github.com/open-telemetry/opentelemetry-python/pull/2763/commits/a6324d891365f61e3631dabd68f3321482f15c51

I have modified the offending line with this and it works:

from time import time_ns as _time_ns

Issue Type

Bug Report

Component Name

community.general.opentelemetry

Ansible Version

$ ansible --version
ansible [core 2.12.2]
  config file = /var/opt/ansible/ansible.cfg
  configured module search path = ['/apps/users/***/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.8/site-packages/ansible
  ansible collection location = /apps/users/***/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.8.12 (default, Sep 16 2021, 10:46:05) [GCC 8.5.0 20210514 (Red Hat 8.5.0-3)]
  jinja version = 2.10.3
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general

# /usr/lib/python3.8/site-packages/ansible_collections
Collection        Version
----------------- -------
community.general 4.5.0

# /usr/share/ansible/collections/ansible_collections
Collection        Version
----------------- -------
community.general 5.6.0

Configuration

$ ansible-config dump --only-changed
CALLBACKS_ENABLED(/var/opt/ansible/ansible.cfg) = ['community.general.opentelemetry']
DEFAULT_FORKS(/var/opt/ansible/ansible.cfg) = 20
DEFAULT_LOG_PATH(/var/opt/ansible/ansible.cfg) = /var/opt/ansible/log/ansible.log
DEFAULT_ROLES_PATH(/var/opt/ansible/ansible.cfg) = ['/apps/users/***/.ansible/roles', '/usr/share/ansible/roles', '/etc/ansible/roles', '/var/opt/ansible/roles']
DEFAULT_VAULT_PASSWORD_FILE(/var/opt/ansible/ansible.cfg) = /var/opt/ansible/password_file

OS / Environment

RHEL 8.6

Steps to Reproduce

Install opentelemetry-api, opentelemetry-sdk and opentelemetry-exporter-otlp in version 1.13 and try to run any playbook with -vvvv

Expected Results

No warnings and Opentelemetry callback working

Actual Results

[WARNING]: Skipping callback 'community.general.opentelemetry', unable to load due to: The `opentelemetry-api`, `opentelemetry-exporter-otlp` or `opentelemetry-sdk` must be installed to use this plugin

Code of Conduct

  • [X] I agree to follow the Ansible Code of Conduct

cpiment avatar Sep 27 '22 08:09 cpiment

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot avatar Sep 27 '22 08:09 ansibullbot

cc @v1v click here for bot help

ansibullbot avatar Sep 27 '22 08:09 ansibullbot

As far as I see time_ns is supported from 3.7 as per https://docs.python.org/3/library/time.html#time.time_ns

While 3.7 is the lowers supported python version as per -> https://devguide.python.org/versions/#supported-versions

Though, there might be people using older versions of python. I don't know if we wanna support that particular use case, I'd inclined to use this proposal here so python 3.7 is the minimal version required so in addition to support > 1.13.0.

I might need some guidelines regarding what's normally the best approach to support, different lib version and different pythons versions in this project

v1v avatar Sep 27 '22 09:09 v1v

Hi @v1v , thanks for your response

I'm testing this in a RHEL 8.6 with ansible 5.4 (EPEL version, ansible-core 2.12). This version of ansible has minimum python version of 3.8 (https://docs.ansible.com/ansible/latest/roadmap/ROADMAP_2_12.html), which is the one I'm using, I think that's why the workaround of importing time_ns from time works, but it is the same thing that the Opentelemetry team has done in their code. In the PR where they removed util._time they talk about removing support for Python 3.6:

https://github.com/open-telemetry/opentelemetry-python/pull/2763

cpiment avatar Sep 27 '22 10:09 cpiment

I might need some guidelines regarding what's normally the best approach to support, different lib version and different pythons versions in this project

Since we stick to semantic versioning, you cannot drop support for versions of a library that did work before in the same major release of this collection. Also we require a deprecation period for dropping support for older versions.

The minimum Python version supported by community.general 5.x.y on the controller side is still Python 2.7, since ansible-core 2.11 does still work with Python 2.7.

One way to make this work is basically do the same thing as util._time was doing, i.e. import time_ns from time if available (i.e. no ImportError) and define a workaround if not.

felixfontein avatar Sep 28 '22 20:09 felixfontein

I've just created https://github.com/ansible-collections/community.general/pull/5342 with a similar proposal as discussed here

v1v avatar Oct 06 '22 20:10 v1v