close
Skip to content

Log stream/topic already exists as debug events.#604

Merged
hubcio merged 1 commit into
apache:masterfrom
gibbz00:already_created_log_levels
Jan 26, 2024
Merged

Log stream/topic already exists as debug events.#604
hubcio merged 1 commit into
apache:masterfrom
gibbz00:already_created_log_levels

Conversation

@gibbz00
Copy link
Copy Markdown
Contributor

@gibbz00 gibbz00 commented Jan 25, 2024

To summarize what was discussed on discord:

Attempting to create topics and streams that have already been created currently logs at the error level. For example:

2024-01-25T15:27:32Z ERROR  Received an invalid response with status: 1012 (stream_name_already_exists).

This may happen as a result of users simply to always create a topic upon producer/consumer initialization, as a means to avoid topic/stream not found error handling. One fix would be to require users to check topic existences first, but this would often result in two request being done rather than just one.

Users already need to explicitly ignore this "error" in the SDK, so I think it's ok to simply increase the corresponding log levels to debug. It also makes things easier for users that don't want to filter away all error layer events just to get rid of those.

Currently, however, all logging is handled centrally by handle_response(), and for each transport backend. This makes it hard to individually tune log levels. This PR solves the tuning by using a conditional for the TCP backend only, but it's a solution that won't scale well at all. Got the go-ahead for this anyway, with the mutual understanding it's merely a temporary change to a solution that would require greater overhaul of the SDK ✌️

@hubcio hubcio added sdk Change related to sdk (client) API and removed sdk Change related to sdk (client) API labels Jan 25, 2024
@gibbz00 gibbz00 force-pushed the already_created_log_levels branch from e296908 to 94daf93 Compare January 25, 2024 17:05
@spetz
Copy link
Copy Markdown
Contributor

spetz commented Jan 25, 2024

Maybe this could be extended to all existing (currently 8) AlreadyExists errors? Also, it seems there are some unused imports. Other than that, looks good!

@gibbz00 gibbz00 force-pushed the already_created_log_levels branch from 94daf93 to 595799d Compare January 25, 2024 18:02
@gibbz00
Copy link
Copy Markdown
Contributor Author

gibbz00 commented Jan 25, 2024

Why not! Fixed 😊

@spetz
Copy link
Copy Markdown
Contributor

spetz commented Jan 25, 2024

Great, there are still some failing tests for the examples, most likely due to the change of output logs.

@gibbz00 gibbz00 force-pushed the already_created_log_levels branch 2 times, most recently from a5a861a to bba95c2 Compare January 26, 2024 09:31
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 26, 2024

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (0534cdf) 74.49% compared to head (bba95c2) 74.50%.

Files Patch % Lines
sdk/src/tcp/client.rs 72.22% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #604      +/-   ##
==========================================
+ Coverage   74.49%   74.50%   +0.01%     
==========================================
  Files         316      316              
  Lines       23985    23998      +13     
  Branches    23985    23998      +13     
==========================================
+ Hits        17867    17880      +13     
- Misses       4325     4330       +5     
+ Partials     1793     1788       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@T1B0
Copy link
Copy Markdown
Contributor

T1B0 commented Jan 26, 2024

<3 better error message 👍

@hubcio hubcio merged commit 8411021 into apache:master Jan 26, 2024
spetz pushed a commit that referenced this pull request Sep 27, 2025
1. Aligned Error Codes and Information with the Rust SDK
2. Refactored IggyError: Discarded Sentinel Errors, Introduced Code(err
error) Helper.
3. Updated Tests
4. Removed the Logic for Ignoring xxxAlreadyExists Errors, as referenced in [PR #604](#604).
hageshiame pushed a commit to hageshiame/iggy that referenced this pull request Nov 7, 2025
1. Aligned Error Codes and Information with the Rust SDK
2. Refactored IggyError: Discarded Sentinel Errors, Introduced Code(err
error) Helper.
3. Updated Tests
4. Removed the Logic for Ignoring xxxAlreadyExists Errors, as referenced in [PR apache#604](apache#604).
ryerraguntla pushed a commit to ryerraguntla/iggy_for_influx that referenced this pull request Mar 13, 2026
1. Aligned Error Codes and Information with the Rust SDK
2. Refactored IggyError: Discarded Sentinel Errors, Introduced Code(err
error) Helper.
3. Updated Tests
4. Removed the Logic for Ignoring xxxAlreadyExists Errors, as referenced in [PR #604](apache/iggy#604).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants