Skip to content

feat(multiplex): Detect duplicated clients #723

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

Merged
merged 2 commits into from
Mar 2, 2023

Conversation

erezrokah
Copy link
Member

@erezrokah erezrokah commented Mar 1, 2023

Summary

Related to #713. This PR detects duplicated client IDs, sends a sentry report when that happens and also logs a warning.
It not this does not skip those clients (yet). I'll do a follow up based on the sentry data as we can have duplicate clients due to:

  1. User errors creating a configuration that generates duplicated clients like in fix(azure): Ensure spec subscriptions are unique cloudquery#8099
  2. Plugins bug, not implementing the ID method correctly

Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@erezrokah erezrokah requested a review from yevgenypats as a code owner March 1, 2023 13:27
@erezrokah erezrokah changed the title feat(multiplex): Detect multiple clients feat(multiplex): Detect duplicated clients Mar 1, 2023
@github-actions github-actions bot added feat and removed feat labels Mar 1, 2023
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 40.00% and project coverage change: -0.02 ⚠️

Comparison is base (b2b8de9) 47.17% compared to head (c02c354) 47.16%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #723      +/-   ##
==========================================
- Coverage   47.17%   47.16%   -0.02%     
==========================================
  Files          70       70              
  Lines        6849     6859      +10     
==========================================
+ Hits         3231     3235       +4     
- Misses       3167     3172       +5     
- Partials      451      452       +1     
Impacted Files Coverage Δ
plugins/source/scheduler_dfs.go 68.67% <40.00%> (-1.84%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link

github-actions bot commented Mar 1, 2023

⏱️ Benchmark results

Comparing with b2b8de9

  • DefaultConcurrencyDFS-2 resources/s: 10,544 ⬇️ 8.36% decrease vs. b2b8de9
  • DefaultConcurrencyRoundRobin-2 resources/s: 11,743 ⬆️ 3.90% increase vs. b2b8de9
  • Glob-2 ns/op: 185.2 ⬇️ 4.00% decrease vs. b2b8de9
  • TablesWithChildrenDFS-2 resources/s: 29,151 ⬇️ 3.78% decrease vs. b2b8de9
  • TablesWithChildrenRoundRobin-2 resources/s: 27,708 ⬇️ 8.35% decrease vs. b2b8de9
  • TablesWithRateLimitingDFS-2 resources/s: 28.47 ⬆️ 0.11% increase vs. b2b8de9
  • TablesWithRateLimitingRoundRobin-2 resources/s: 806.2 ⬇️ 1.71% decrease vs. b2b8de9
  • BufferedScanner-2 ns/op: 9.387 ⬇️ 0.13% decrease vs. b2b8de9
  • LogReader-2 ns/op: 30.68 ⬇️ 0.03% decrease vs. b2b8de9

@kodiakhq kodiakhq bot merged commit dfb039d into cloudquery:main Mar 2, 2023
kodiakhq bot pushed a commit that referenced this pull request Mar 2, 2023
🤖 I have created a release *beep* *boop*
---


## [1.41.0](v1.40.0...v1.41.0) (2023-03-02)


### Features

* Deterministic _cq_id ([#712](#712)) ([2e7ad2c](2e7ad2c))
* **multiplex:** Detect duplicated clients ([#723](#723)) ([dfb039d](dfb039d))


### Bug Fixes

* Cleanup code ([#710](#710)) ([963f03c](963f03c))
* **deps:** Update golang.org/x/exp digest to c95f2b4 ([#718](#718)) ([de52c10](de52c10))
* **deps:** Update google.golang.org/genproto digest to 9b19f0b ([#719](#719)) ([ecfddea](ecfddea))
* **deps:** Update module github.com/rivo/uniseg to v0.4.4 ([#720](#720)) ([0da69b6](0da69b6))
* **deps:** Update module github.com/stretchr/testify to v1.8.2 ([#721](#721)) ([19c0742](19c0742))
* **pk:** Skip filter for no PK ([#709](#709)) ([d0c2e26](d0c2e26))
* **types-json:** Disable HTML escaping during JSON marshalling ([#714](#714)) ([2f6f1d8](2f6f1d8))
* **types-timestamp:** Ensure timestamp is UTC ([#716](#716)) ([bb33629](bb33629))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@erezrokah erezrokah deleted the feat/detect_multiple_clients branch March 12, 2023 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants