Skip to content

Remove redundant span stack handling and error logging#231

Merged
rapids-bot[bot] merged 1 commit intoNVIDIA:developfrom
dnandakumar-nv:otel-span-bug
May 9, 2025
Merged

Remove redundant span stack handling and error logging#231
rapids-bot[bot] merged 1 commit intoNVIDIA:developfrom
dnandakumar-nv:otel-span-bug

Conversation

@dnandakumar-nv
Copy link
Contributor

@dnandakumar-nv dnandakumar-nv commented May 9, 2025

The redundant span stack error logging and unused pop operation were removed to streamline the code. These changes reduce unnecessary noise in the logs and improve the clarity of span management logic.

Description

The redundant span stack error logging and unused pop operation were removed to streamline the code. These changes reduce unnecessary noise in the logs and improve the clarity of span management logic.

Also prevents bug where spans are not logged if multiple subspans use the same parent span sequentially. Previously, as soon as a child span was closed, the parent span was popped from the span stack <- Bug

Closes #232

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
    • Any contribution which contains commits that are not Signed-Off will not be accepted.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

The redundant span stack error logging and unused pop operation were removed to streamline the code. These changes reduce unnecessary noise in the logs and improve the clarity of span management logic.

Signed-off-by: dnandakumar-nv <168006707+dnandakumar-nv@users.noreply.github.com>
@copy-pr-bot
Copy link

copy-pr-bot bot commented May 9, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@dnandakumar-nv dnandakumar-nv added bug Something isn't working non-breaking Non-breaking change labels May 9, 2025
@dnandakumar-nv dnandakumar-nv requested review from a team and AnuradhaKaruppiah May 9, 2025 02:44
@dnandakumar-nv
Copy link
Contributor Author

/ok to test c112eb6

@AnuradhaKaruppiah AnuradhaKaruppiah requested a review from Copilot May 9, 2025 02:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes redundant error logging and an unnecessary pop operation from the span stack to streamline span management and fix a bug where multiple subspans sharing the same parent lead to premature parent removal.

  • Removed error logging for unclosed spans in the cleanup method
  • Eliminated the pop operation for subspans in the end event handler

@AnuradhaKaruppiah
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit 38620c6 into NVIDIA:develop May 9, 2025
10 checks passed
yczhang-nv pushed a commit to yczhang-nv/NeMo-Agent-Toolkit that referenced this pull request May 9, 2025
The redundant span stack error logging and unused pop operation were removed to streamline the code. These changes reduce unnecessary noise in the logs and improve the clarity of span management logic.

Also prevents bug where spans are not logged if multiple subspans use the same parent span sequentially. Previously, as soon as a child span was closed, the parent span was popped from the span stack <- Bug

Closes NVIDIA#232

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Dhruv Nandakumar (https://github.com/dnandakumar-nv)

Approvers:
  - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah)

URL: NVIDIA#231
Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
yczhang-nv pushed a commit to yczhang-nv/NeMo-Agent-Toolkit that referenced this pull request May 9, 2025
The redundant span stack error logging and unused pop operation were removed to streamline the code. These changes reduce unnecessary noise in the logs and improve the clarity of span management logic.

Also prevents bug where spans are not logged if multiple subspans use the same parent span sequentially. Previously, as soon as a child span was closed, the parent span was popped from the span stack <- Bug

Closes NVIDIA#232

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Dhruv Nandakumar (https://github.com/dnandakumar-nv)

Approvers:
  - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah)

URL: NVIDIA#231
Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
ericevans-nv pushed a commit to ericevans-nv/agent-iq that referenced this pull request Jun 3, 2025
The redundant span stack error logging and unused pop operation were removed to streamline the code. These changes reduce unnecessary noise in the logs and improve the clarity of span management logic.

Also prevents bug where spans are not logged if multiple subspans use the same parent span sequentially. Previously, as soon as a child span was closed, the parent span was popped from the span stack <- Bug

Closes NVIDIA#232

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Dhruv Nandakumar (https://github.com/dnandakumar-nv)

Approvers:
  - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah)

URL: NVIDIA#231
Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.com>
ericevans-nv pushed a commit to ericevans-nv/agent-iq that referenced this pull request Jun 3, 2025
The redundant span stack error logging and unused pop operation were removed to streamline the code. These changes reduce unnecessary noise in the logs and improve the clarity of span management logic.

Also prevents bug where spans are not logged if multiple subspans use the same parent span sequentially. Previously, as soon as a child span was closed, the parent span was popped from the span stack <- Bug

Closes NVIDIA#232

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Dhruv Nandakumar (https://github.com/dnandakumar-nv)

Approvers:
  - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah)

URL: NVIDIA#231
Signed-off-by: Eric Evans <194135482+ericevans-nv@users.noreply.github.com>
AnuradhaKaruppiah pushed a commit to AnuradhaKaruppiah/oss-agentiq that referenced this pull request Aug 4, 2025
The redundant span stack error logging and unused pop operation were removed to streamline the code. These changes reduce unnecessary noise in the logs and improve the clarity of span management logic.

Also prevents bug where spans are not logged if multiple subspans use the same parent span sequentially. Previously, as soon as a child span was closed, the parent span was popped from the span stack <- Bug

Closes NVIDIA#232 

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Dhruv Nandakumar (https://github.com/dnandakumar-nv)

Approvers:
  - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah)

URL: NVIDIA#231
scheckerNV pushed a commit to scheckerNV/aiq-factory-reset that referenced this pull request Aug 22, 2025
The redundant span stack error logging and unused pop operation were removed to streamline the code. These changes reduce unnecessary noise in the logs and improve the clarity of span management logic.

Also prevents bug where spans are not logged if multiple subspans use the same parent span sequentially. Previously, as soon as a child span was closed, the parent span was popped from the span stack <- Bug

Closes NVIDIA#232 

## By Submitting this PR I confirm:
- I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/AIQToolkit/blob/develop/docs/source/advanced/contributing.md).
- We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
  - Any contribution which contains commits that are not Signed-Off will not be accepted.
- When the PR is ready for review, new or existing tests cover these changes.
- When the PR is ready for review, the documentation is up to date with these changes.

Authors:
  - Dhruv Nandakumar (https://github.com/dnandakumar-nv)

Approvers:
  - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah)

URL: NVIDIA#231
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: alert-triage-agent is throwing WARNING "No parent span found for step ef10b8cb-bee4-44dd-af7b-cad3bf3f615c"

3 participants