Skip to content

ref(span-buffer): Remove flusher and buffer logger options#115487

Merged
untitaker merged 1 commit into
masterfrom
untitaker/remove-flusher-logger-options
May 18, 2026
Merged

ref(span-buffer): Remove flusher and buffer logger options#115487
untitaker merged 1 commit into
masterfrom
untitaker/remove-flusher-logger-options

Conversation

@untitaker
Copy link
Copy Markdown
Member

Summary

  • Remove spans.buffer.flusher-cumulative-logger-enabled option and FlusherLogger class
  • Remove spans.buffer.flusher.log-flushed-segments option and associated logging
  • Remove spans.buffer.evalsha-cumulative-logger-enabled option and BufferLogger class

These logging options were used for debugging during development but are no longer needed.

Refs STREAM-881

@untitaker untitaker requested review from a team as code owners May 13, 2026 16:03
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 13, 2026

STREAM-881

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 13, 2026
Copy link
Copy Markdown
Contributor

@fpacifici fpacifici left a comment

Choose a reason for hiding this comment

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

IF you remove the logger you also have to remove the script to parse them

But, as I said below, I would not remove the buffer logger till we do not get rid of OTLP enrichment. We still have possible failure modes where specific traces can become problematic. Having the log and the log analyzer allow us to quickly identify the trace.

return last_log_time


class BufferLogger:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would not remove the buffer logger till we do not get rid of OTLP enrichment altogether.
There are still possible failure modes where we would want to identify a problematic trace.

Ok to remove the flusher one

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

removed the flusher logger only.

@untitaker untitaker force-pushed the untitaker/remove-flusher-logger-options branch from 37edf06 to d9ae398 Compare May 18, 2026 15:06
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d9ae398. Configure here.

Comment thread tests/sentry/spans/test_buffer.py
Remove the FlusherLogger and its associated options while keeping
the BufferLogger for identifying problematic traces during OTLP
enrichment.
@untitaker untitaker force-pushed the untitaker/remove-flusher-logger-options branch from d9ae398 to 3c2ba59 Compare May 18, 2026 15:12
@untitaker untitaker merged commit 2592342 into master May 18, 2026
114 of 117 checks passed
@untitaker untitaker deleted the untitaker/remove-flusher-logger-options branch May 18, 2026 16:07
@untitaker untitaker added the Trigger: Revert Add to a merged PR to revert it (skips CI) label May 21, 2026
@untitaker
Copy link
Copy Markdown
Member Author

have to revert this to investigate another instance of a single redis shard running hot

@getsentry-bot
Copy link
Copy Markdown
Contributor

revert failed (conflict? already reverted?) -- check the logs

@github-actions
Copy link
Copy Markdown
Contributor

Backend Test Failures

Failures on 2592342 in this run:

tests/snuba/api/endpoints/test_project_trace_item_details.py::ProjectTraceItemDetailsEndpointTest::test_details_exposes_arrayslog
[gw0] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/snuba/api/endpoints/test_project_trace_item_details.py:231: in test_details_exposes_arrays
    assert by_name["stack.filename"]["type"] == "array"
           ^^^^^^^^^^^^^^^^^^^^^^^^^
E   KeyError: 'stack.filename'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components Trigger: Revert Add to a merged PR to revert it (skips CI)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants