fix(opentelemetry): adjust default value for `store_spans_in_file` causing traces to be produced to a file named `None` #8741

wilfriedroset · 2024-08-11T20:24:00Z

SUMMARY

The commit 5f48193 introduced store_spans_in_file with the default
value None as a string. This causes the value of store_spans_in_file
to be a not empty string, value=None as a string and not a null value.
The rest of the code check if the store_spans_in_file is not null which
squeezes the rest of the code. The following commit set the default
value as an empty string.

Fixes (hopefully) #8566

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

community.general.opentelemetry

ADDITIONAL INFORMATION

I've used the same reproducer as #8321 with git bisect

5f481939d4558e1ae61459cee2a4aecb2f8a7207 is the first bad commit
commit 5f481939d4558e1ae61459cee2a4aecb2f8a7207 (HEAD)
Author: Victor Martinez <[email protected]>
Date:   Sun May 19 20:48:49 2024 +0200

    feat(opentelemetry): support flag to export spans in a given file (#8363)

    * opentelemetry: support flag to create output file

    this is only to help with adding unit tests

    * refactor and rename

    * changelog

    * rename

    * fix linting

 changelogs/fragments/8363-opentelemetry-export-to-a-file.yml |  2 ++
 plugins/callback/opentelemetry.py                            | 48 ++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 8 deletions(-)
 create mode 100644 changelogs/fragments/8363-opentelemetry-export-to-a-file.yml

…using traces to be produced to a file named `None` The commit 5f48193 introduced `store_spans_in_file` with the default value `None` as a string. This causes the value of `store_spans_in_file` to be a not empty string, value=None as a string and not a null value. The rest of the code check if the store_spans_in_file is not null which squeezes the rest of the code. The following commit set the default value as an empty string. Signed-off-by: Wilfried Roset <[email protected]>

wilfriedroset · 2024-08-11T20:27:02Z

ping @moserke @rojon8 @cervajs as you have stumbled on the bug. I would appreciate if you could test this small change.

ping @v1v @russoz for the review 🙏🏾.

plugins/callback/opentelemetry.py

felixfontein

Thanks for your contribution!

plugins/callback/opentelemetry.py

changelogs/fragments/8741-fix-opentelemetry-callback.yml

Signed-off-by: Wilfried Roset <[email protected]>

felixfontein · 2024-08-12T05:41:57Z

I don't know whether this fixes #8566, but this change should be merged nonetheless since writing to a file None is definitely wrong. I've unlinked the issue from this PR, let's close the issue if testing by the others confirms this PR fixes it. I'm merging this PR now so it will be included in today's release.

patchback · 2024-08-12T05:42:09Z

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/73b54139d65c94aac5f5b4ba251afa2d676cb12e/pr-8741

Backported as #8751

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

…using traces to be produced to a file named `None` (#8741) * fix(opentelemetry): adjust default value for `store_spans_in_file` causing traces to be produced to a file named `None` The commit 5f48193 introduced `store_spans_in_file` with the default value `None` as a string. This causes the value of `store_spans_in_file` to be a not empty string, value=None as a string and not a null value. The rest of the code check if the store_spans_in_file is not null which squeezes the rest of the code. The following commit set the default value as an empty string. Signed-off-by: Wilfried Roset <[email protected]> * fix(opentelemetry): No default value is better, reword changelog Signed-off-by: Wilfried Roset <[email protected]> --------- Signed-off-by: Wilfried Roset <[email protected]> (cherry picked from commit 73b5413)

felixfontein · 2024-08-12T05:43:19Z

@wilfriedroset thanks for fixing this!

…ault value for `store_spans_in_file` causing traces to be produced to a file named `None` (#8751) fix(opentelemetry): adjust default value for `store_spans_in_file` causing traces to be produced to a file named `None` (#8741) * fix(opentelemetry): adjust default value for `store_spans_in_file` causing traces to be produced to a file named `None` The commit 5f48193 introduced `store_spans_in_file` with the default value `None` as a string. This causes the value of `store_spans_in_file` to be a not empty string, value=None as a string and not a null value. The rest of the code check if the store_spans_in_file is not null which squeezes the rest of the code. The following commit set the default value as an empty string. Signed-off-by: Wilfried Roset <[email protected]> * fix(opentelemetry): No default value is better, reword changelog Signed-off-by: Wilfried Roset <[email protected]> --------- Signed-off-by: Wilfried Roset <[email protected]> (cherry picked from commit 73b5413) Co-authored-by: Wilfried ROSET <[email protected]>

…using traces to be produced to a file named `None` (ansible-collections#8741) * fix(opentelemetry): adjust default value for `store_spans_in_file` causing traces to be produced to a file named `None` The commit 5f48193 introduced `store_spans_in_file` with the default value `None` as a string. This causes the value of `store_spans_in_file` to be a not empty string, value=None as a string and not a null value. The rest of the code check if the store_spans_in_file is not null which squeezes the rest of the code. The following commit set the default value as an empty string. Signed-off-by: Wilfried Roset <[email protected]> * fix(opentelemetry): No default value is better, reword changelog Signed-off-by: Wilfried Roset <[email protected]> --------- Signed-off-by: Wilfried Roset <[email protected]>

wilfriedroset force-pushed the fix-8566 branch from ac31e9b to 045079c Compare August 11, 2024 20:24

ansibullbot added bug This issue/PR relates to a bug callback callback plugin plugins plugin (any type) labels Aug 11, 2024

wilfriedroset commented Aug 11, 2024

View reviewed changes

plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved

felixfontein reviewed Aug 11, 2024

View reviewed changes

plugins/callback/opentelemetry.py Outdated Show resolved Hide resolved

changelogs/fragments/8741-fix-opentelemetry-callback.yml Outdated Show resolved Hide resolved

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-9 Automatically create a backport for the stable-9 branch labels Aug 11, 2024

fix(opentelemetry): No default value is better, reword changelog

5a642e1

Signed-off-by: Wilfried Roset <[email protected]>

felixfontein merged commit 73b5413 into ansible-collections:main Aug 12, 2024
147 checks passed

patchback bot mentioned this pull request Aug 12, 2024

[PR #8741/73b54139 backport][stable-9] fix(opentelemetry): adjust default value for store_spans_in_file causing traces to be produced to a file named None #8751

Merged

felixfontein mentioned this pull request Aug 12, 2024

Opentelemetry stops sending traces with 9.0.0 #8566

Closed

1 task

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Aug 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(opentelemetry): adjust default value for `store_spans_in_file` causing traces to be produced to a file named `None` #8741

fix(opentelemetry): adjust default value for `store_spans_in_file` causing traces to be produced to a file named `None` #8741

wilfriedroset commented Aug 11, 2024 •

edited by felixfontein

Loading

wilfriedroset commented Aug 11, 2024

felixfontein left a comment

felixfontein commented Aug 12, 2024

patchback bot commented Aug 12, 2024 •

edited

Loading

felixfontein commented Aug 12, 2024

fix(opentelemetry): adjust default value for store_spans_in_file causing traces to be produced to a file named None #8741

fix(opentelemetry): adjust default value for store_spans_in_file causing traces to be produced to a file named None #8741

Conversation

wilfriedroset commented Aug 11, 2024 • edited by felixfontein Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

wilfriedroset commented Aug 11, 2024

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein commented Aug 12, 2024

patchback bot commented Aug 12, 2024 • edited Loading

Backport to stable-9: 💚 backport PR created

felixfontein commented Aug 12, 2024

fix(opentelemetry): adjust default value for `store_spans_in_file` causing traces to be produced to a file named `None` #8741

fix(opentelemetry): adjust default value for `store_spans_in_file` causing traces to be produced to a file named `None` #8741

wilfriedroset commented Aug 11, 2024 •

edited by felixfontein

Loading

patchback bot commented Aug 12, 2024 •

edited

Loading