Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add opentelemetry callback plugin #3091

Conversation

v1v
Copy link
Contributor

@v1v v1v commented Jul 27, 2021

SUMMARY

Send distributed traces for the ansible runs with OpenTelemetry

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

plugins/callback/opentelemetry.py

ADDITIONAL INFORMATION

You can configure this plugin and then run your playbook with the below environment variables. Those environment variables are the ones defined by the opentelemetry project (see https://opentelemetry-python.readthedocs.io/en/latest/exporter/otlp/otlp.html)

In order to configure this plugins is requried to install the below python libs:

pip3 install opentelemetry-api opentelemetry-sdk opentelemetry-exporter-otlp

Then you can start sending traces to your OTEL endpoint:

$ OTEL_SERVICE_NAME='ansible-otel' \
   OTEL_EXPORTER_OTLP_ENDPOINT='<your_otlp_endpoint>' \
   OTEL_EXPORTER_OTLP_HEADERS='authorization=Bearer <your_token>' \
   ansible-playbook <your_playbook>

Or even you can see the distributed traces in the console:

$ OPENTELEMETRY_CONSOLE_OUTPUT=true \
   ansible-playbook <your_playbook>

This is vendor agnostic, as long as the OpenTelemetry is supported those traces are sent, for instance, when running the molecule test the converge stage produced the below traces for one of our roles.

image

Molecule was configured with something like:

provisioner:
  name: ansible
  env:
    JUNIT_OUTPUT_DIR: $MOLECULE_SCENARIO_DIRECTORY/test-results
  config_options:
    defaults:
      callback_plugins: $MOLECULE_SCENARIO_DIRECTORY/../../../../callback_plugins
      callback_whitelist: opentelemetry
Questions
  • Can I add some ITs to help?
Tasks
  • Add tests
  • Verify changes locally.
  • Verify changes in a staging environment.
Follow ups

The below follow ups will be tackle in future PRs after merging this

  1. Add context propagation
  2. Handle task failures as errors

@ansibullbot ansibullbot added affects_2.10 callback callback plugin community_review needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_triage new_contributor Help guide this first time contributor new_plugin New plugin plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging labels Jul 27, 2021
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! I've added some first comments.

Please also check the failing sanity tests.

Also please note that all new plugins and modules must come with tests.

plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
@v1v v1v marked this pull request as ready for review September 2, 2021 16:24
@v1v
Copy link
Contributor Author

v1v commented Sep 3, 2021

/rebuild_failed

@codecov
Copy link

codecov bot commented Sep 3, 2021

Codecov Report

Merging #3091 (3502f3b) into main (118c040) will decrease coverage by 0.17%.
The diff coverage is n/a.

❗ Current head 3502f3b differs from pull request most recent head deb84a6. Consider uploading reports for the commit deb84a6 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3091      +/-   ##
==========================================
- Coverage   43.56%   43.38%   -0.18%     
==========================================
  Files         928      923       -5     
  Lines       89375    89069     -306     
  Branches    16284    16265      -19     
==========================================
- Hits        38933    38646     -287     
+ Misses      48737    48720      -17     
+ Partials     1705     1703       -2     
Impacted Files Coverage Δ
plugins/modules/system/pids.py 78.18% <0.00%> (-8.43%) ⬇️
plugins/modules/files/iso_create.py 75.23% <0.00%> (-3.81%) ⬇️
plugins/modules/storage/zfs/zpool_facts.py 28.07% <0.00%> (-3.31%) ⬇️
plugins/modules/net_tools/nmcli.py 79.21% <0.00%> (-1.47%) ⬇️
...modules/source_control/gitlab/test_gitlab_group.py 78.57% <0.00%> (-1.16%) ⬇️
tests/unit/plugins/modules/net_tools/test_nmcli.py 99.89% <0.00%> (-0.01%) ⬇️
plugins/inventory/linode.py 47.11% <0.00%> (ø)
...it/plugins/modules/source_control/gitlab/gitlab.py 83.98% <0.00%> (ø)
...gins/modules/database/misc/test_redis_data_info.py
plugins/module_utils/redis.py
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd63da6...deb84a6. Read the comment docs.

Copy link

@basepi basepi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome! 👍

@ansibullbot ansibullbot added tests tests unit tests/unit labels Sep 4, 2021
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved
@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Sep 6, 2021
@ansibullbot
Copy link
Collaborator

@v1v this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added merge_commit This PR contains at least one merge commit. Please resolve! and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 6, 2021
@v1v v1v force-pushed the feature/add-opentelemetry-callback-plugin branch from 4130319 to deb84a6 Compare September 14, 2021 08:02
@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed merge_commit This PR contains at least one merge commit. Please resolve! needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 14, 2021
@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 14, 2021
@felixfontein felixfontein merged commit 517570a into ansible-collections:main Sep 14, 2021
@patchback
Copy link

patchback bot commented Sep 14, 2021

Backport to stable-3: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-3/517570a64fcbbfdad5704f88c63eabb4cf74a84d/pr-3091

Backported as #3373

🤖 @patchback
I'm built with octomachinery and
my source is open — https:/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Sep 14, 2021
* Add opentelemetry callback plugin

* Apply suggestions from code review

Co-authored-by: Felix Fontein <[email protected]>

* Formatting (text), booleans and renamed env variables

* This should be done in a future release

* Remove insecure in favour of the OTEL env variable. Add descriptions

* Use OpenTelemetrySource

* Move generate_distributed_traces

* Move update_span_data and set_span_attribute

* Move finish_task

* Move start_task

* Refactor to support UTs

* Add first UT

* Fix codestyle

* opentelemetry callback entry in the botmeta

* Fix linting

* Fix signature

* Mock methods

* Use MagicMock

* Mock the methods

* UT for transform_to_boolean_or_default

* Fix linting

* Set test data

* Mock _time_ns

* Exclude tests for python <= 3.6

* Remove obsoleted setup task type configuration

* Remove unused docs

* Apply suggestions from code review

Co-authored-by: Felix Fontein <[email protected]>

* Fix docs

* unrequired logic that was originally took from https:/ansible/ansible/blob/devel/lib/ansible/plugins/callback/junit.py\#L226

* Use raise_from for the required dependencies

* Fix linting

* Add requirements for the UTs

* add missing dependency for the opentelemetry plugin in the UTs

* Add ANSIBLE_ prefix for the ansible specific options

* Add more context in the docs and remove duplicated docs

* As suggested in the code review

* Verify if the OTEL env variables for the endpoint were set

* Fix docs typo

* Fix linting

* Revert "Fix linting"

This reverts commit 3a54c827c5472553a6baf5598bc76a0f63f020c1.

* Revert "Verify if the OTEL env variables for the endpoint were set"

This reverts commit cab9d8648899c28c0345745690c4ec7a41f7e680.

* Remove console_output as suggested

* Apply suggestions from code review

Co-authored-by: flowerysong <[email protected]>

* Delegate the definition of OTEL_EXPORTER_OTLP_INSECURE to the user

* Move definitions above, close to the class that uses them

Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: flowerysong <[email protected]>
(cherry picked from commit 517570a)
@felixfontein
Copy link
Collaborator

@v1v thanks for contributing this plugin!
@basepi @flowerysong thanks for reviewing this!

felixfontein pushed a commit that referenced this pull request Sep 15, 2021
* Add opentelemetry callback plugin

* Apply suggestions from code review

Co-authored-by: Felix Fontein <[email protected]>

* Formatting (text), booleans and renamed env variables

* This should be done in a future release

* Remove insecure in favour of the OTEL env variable. Add descriptions

* Use OpenTelemetrySource

* Move generate_distributed_traces

* Move update_span_data and set_span_attribute

* Move finish_task

* Move start_task

* Refactor to support UTs

* Add first UT

* Fix codestyle

* opentelemetry callback entry in the botmeta

* Fix linting

* Fix signature

* Mock methods

* Use MagicMock

* Mock the methods

* UT for transform_to_boolean_or_default

* Fix linting

* Set test data

* Mock _time_ns

* Exclude tests for python <= 3.6

* Remove obsoleted setup task type configuration

* Remove unused docs

* Apply suggestions from code review

Co-authored-by: Felix Fontein <[email protected]>

* Fix docs

* unrequired logic that was originally took from https:/ansible/ansible/blob/devel/lib/ansible/plugins/callback/junit.py\#L226

* Use raise_from for the required dependencies

* Fix linting

* Add requirements for the UTs

* add missing dependency for the opentelemetry plugin in the UTs

* Add ANSIBLE_ prefix for the ansible specific options

* Add more context in the docs and remove duplicated docs

* As suggested in the code review

* Verify if the OTEL env variables for the endpoint were set

* Fix docs typo

* Fix linting

* Revert "Fix linting"

This reverts commit 3a54c827c5472553a6baf5598bc76a0f63f020c1.

* Revert "Verify if the OTEL env variables for the endpoint were set"

This reverts commit cab9d8648899c28c0345745690c4ec7a41f7e680.

* Remove console_output as suggested

* Apply suggestions from code review

Co-authored-by: flowerysong <[email protected]>

* Delegate the definition of OTEL_EXPORTER_OTLP_INSECURE to the user

* Move definitions above, close to the class that uses them

Co-authored-by: Felix Fontein <[email protected]>
Co-authored-by: flowerysong <[email protected]>
(cherry picked from commit 517570a)

Co-authored-by: Victor Martinez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
callback callback plugin has_issue needs_triage new_contributor Help guide this first time contributor new_plugin New plugin plugins plugin (any type) tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants