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

NO-SNOW Too many info logs for successfully called put and precommit,… #660

Merged
merged 2 commits into from
Jun 27, 2023

Conversation

sfc-gh-japatel
Copy link
Collaborator

… this will just blow up

  • It's okay for testing but this will blow up the disk space.
  • For reference:
Screenshot 2023-06-22 at 2 44 20 PM

@sfc-gh-tzhang
Copy link
Contributor

… this will just blow up

  • It's okay for testing but this will blow up the disk space.
  • For reference:
Screenshot 2023-06-22 at 2 44 20 PM

I suggest we keep this as INFO, understand the concern here and there're customers that may complain, but this is really important for perf regression investigation and we can't ask customer to simply do DEBUG since there will be too many logs for the SDK

@sfc-gh-tzhang
Copy link
Contributor

… this will just blow up

  • It's okay for testing but this will blow up the disk space.
  • For reference:
Screenshot 2023-06-22 at 2 44 20 PM

I suggest we keep this as INFO, understand the concern here and there're customers that may complain, but this is really important for perf regression investigation and we can't ask customer to simply do DEBUG since there will be too many logs for the SDK

Or we could do some sort of sampling so that we could at least know the trends on how long it would take for the PUT to finish

@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Merging #660 (98e778f) into master (ff10dee) will decrease coverage by 0.10%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #660      +/-   ##
==========================================
- Coverage   87.95%   87.85%   -0.10%     
==========================================
  Files          50       50              
  Lines        4143     4143              
  Branches      450      450              
==========================================
- Hits         3644     3640       -4     
- Misses        330      332       +2     
- Partials      169      171       +2     
Impacted Files Coverage Δ
...m/snowflake/kafka/connector/SnowflakeSinkTask.java 72.19% <100.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sfc-gh-japatel
Copy link
Collaborator Author

… this will just blow up

  • It's okay for testing but this will blow up the disk space.
  • For reference:
Screenshot 2023-06-22 at 2 44 20 PM

I suggest we keep this as INFO, understand the concern here and there're customers that may complain, but this is really important for perf regression investigation and we can't ask customer to simply do DEBUG since there will be too many logs for the SDK

SDK and KC logging are different and can be set at a class level.

com.snowflake=DEBUG,net.snowflake=ERROR

For aws msk it was a bit difficult but we dont have too many customers on it.

@sfc-gh-japatel sfc-gh-japatel merged commit cfcf882 into master Jun 27, 2023
@sfc-gh-japatel sfc-gh-japatel deleted the japatel-no-snow-debug-level branch June 27, 2023 22:10
khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023
khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023
khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023
khsoneji pushed a commit to confluentinc/snowflake-kafka-connector that referenced this pull request Oct 12, 2023
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