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

refactor: return created record from registerGuest #32620

Merged
merged 14 commits into from
Jun 27, 2024

Conversation

ricardogarim
Copy link
Contributor

@ricardogarim ricardogarim commented Jun 18, 2024

Related to OPI-5, PR #72 from Apps.WhatsApp, and PR #771 from Rocket.Chat.Apps-engine.

On the Apps.WhatsApp project, the process of handling new messages and incoming message endpoints has been streamlined. Specifically, there is no need to call getLivechatVisitors after createVisitor when a new message arrives. In endpoints/WhatsAppMessage.post, the defineVisitor function within registerVisitor has been modified to use modify.getCreator().getLivechatCreator().createVisitor() only. The call to read.getLivechatReader().getLivechatVisitors() has been removed since createVisitor now returns the inserted or updated record.

Also, changes were made here at Rocket.Chat project. The createVisitor method in the Livechat bridge has been updated. Within LivechatTyped.registerGuest, a new method on model LivechatVisitors, called updateOneByIdOrToken was added to handle both insert and update operations. This eliminates the need to run an insert followed by an update, optimizing the overall process.

Performance comparison

Use case Avg Duration (ms) Min Duration (ms) Median Duration (ms) Max Duration (ms) 90th Percentile (ms) 95th Percentile (ms)
New visitor BEFORE 426.73 247.68 398.33 945.99 567.71 727.53
New visitor AFTER 381.29 205.85 360.51 761.91 531.72 565.77
Same visitor BEFORE 178.33 84.09 163.78 555.61 257.91 286.1
Same visitor AFTER 164.74 83.6 146.3 446.9 223.83 252.66
K6 test results before changes

Incoming messages from WhatsApp flow with new visitor on every request.

          /\      |‾‾| /‾‾/   /‾‾/   
     /\  /  \     |  |/  /   /  /    
    /  \/    \    |     (   /   ‾‾\  
   /          \   |  |\  \ |  (‾)  | 
  / __________ \  |__| \__\ \_____/ .io

     execution: local
        script: src/incoming-message.js
        output: -

     scenarios: (100.00%) 1 scenario, 1 max VUs, 10m30s max duration (incl. graceful stop):
              * default: 100 iterations shared among 1 VUs (maxDuration: 10m0s, gracefulStop: 30s)


     data_received..................: 76 kB 1.8 kB/s
     data_sent......................: 46 kB 1.1 kB/s
     http_req_blocked...............: avg=64.65µs  min=2µs      med=3µs      max=6.1ms    p(90)=6µs      p(95)=7.04µs  
     http_req_connecting............: avg=5.45µs   min=0s       med=0s       max=545µs    p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=426.21ms min=247.53ms med=398.19ms max=945.79ms p(90)=567.49ms p(95)=727.36ms
       { expected_response:true }...: avg=426.21ms min=247.53ms med=398.19ms max=945.79ms p(90)=567.49ms p(95)=727.36ms
     http_req_failed................: 0.00% ✓ 0       ✗ 100
     http_req_receiving.............: avg=94.16µs  min=49µs     med=76.5µs   max=790µs    p(90)=116.1µs  p(95)=139.89µs
     http_req_sending...............: avg=44.83µs  min=17µs     med=25µs     max=1.45ms   p(90)=40.1µs   p(95)=50.05µs 
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=426.08ms min=247.42ms med=398.1ms  max=945.68ms p(90)=567.38ms p(95)=727.26ms
     http_reqs......................: 100   2.34319/s
     iteration_duration.............: avg=426.73ms min=247.68ms med=398.33ms max=945.99ms p(90)=567.71ms p(95)=727.53ms
     iterations.....................: 100   2.34319/s
     vus............................: 1     min=1     max=1
     vus_max........................: 1     min=1     max=1


running (00m42.7s), 0/1 VUs, 100 complete and 0 interrupted iterations
default ✓ [=======================] 1 VUs  00m42.7s/10m0s  100/100 shared iters

Incoming messages from WhatsApp flow with same visitor on every request.

          /\      |‾‾| /‾‾/   /‾‾/   
     /\  /  \     |  |/  /   /  /    
    /  \/    \    |     (   /   ‾‾\  
   /          \   |  |\  \ |  (‾)  | 
  / __________ \  |__| \__\ \_____/ .io

     execution: local
        script: src/incoming-message.js
        output: -

     scenarios: (100.00%) 1 scenario, 1 max VUs, 10m30s max duration (incl. graceful stop):
              * default: 100 iterations shared among 1 VUs (maxDuration: 10m0s, gracefulStop: 30s)


     data_received..................: 76 kB 4.3 kB/s
     data_sent......................: 46 kB 2.6 kB/s
     http_req_blocked...............: avg=24.53µs  min=2µs     med=3µs      max=2.09ms   p(90)=5µs      p(95)=9.04µs  
     http_req_connecting............: avg=2.38µs   min=0s      med=0s       max=239µs    p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=178.04ms min=83.93ms med=163.62ms max=555.45ms p(90)=257.74ms p(95)=285.86ms
       { expected_response:true }...: avg=178.04ms min=83.93ms med=163.62ms max=555.45ms p(90)=257.74ms p(95)=285.86ms
     http_req_failed................: 0.00% ✓ 0        ✗ 100
     http_req_receiving.............: avg=78.91µs  min=53µs    med=74µs     max=285µs    p(90)=99µs     p(95)=105.09µs
     http_req_sending...............: avg=31.68µs  min=18µs    med=25.5µs   max=524µs    p(90)=34.1µs   p(95)=38.14µs 
     http_req_tls_handshaking.......: avg=0s       min=0s      med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=177.93ms min=83.84ms med=163.51ms max=555.34ms p(90)=257.63ms p(95)=285.74ms
     http_reqs......................: 100   5.606583/s
     iteration_duration.............: avg=178.33ms min=84.09ms med=163.78ms max=555.61ms p(90)=257.91ms p(95)=286.1ms 
     iterations.....................: 100   5.606583/s
     vus............................: 1     min=1      max=1
     vus_max........................: 1     min=1      max=1


running (00m17.8s), 0/1 VUs, 100 complete and 0 interrupted iterations
default ✓ [=======================] 1 VUs  00m17.8s/10m0s  100/100 shared iters
K6 test results after changes

Incoming messages from WhatsApp flow with new visitor on every request.

          /\      |‾‾| /‾‾/   /‾‾/   
     /\  /  \     |  |/  /   /  /    
    /  \/    \    |     (   /   ‾‾\  
   /          \   |  |\  \ |  (‾)  | 
  / __________ \  |__| \__\ \_____/ .io

     execution: local
        script: src/incoming-message.js
        output: -

     scenarios: (100.00%) 1 scenario, 1 max VUs, 10m30s max duration (incl. graceful stop):
              * default: 100 iterations shared among 1 VUs (maxDuration: 10m0s, gracefulStop: 30s)


     data_received..................: 76 kB 2.0 kB/s
     data_sent......................: 47 kB 1.2 kB/s
     http_req_blocked...............: avg=38.97µs  min=2µs      med=3µs      max=3.55ms   p(90)=5µs      p(95)=7µs     
     http_req_connecting............: avg=2.46µs   min=0s       med=0s       max=247µs    p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=381.05ms min=205.64ms med=360.35ms max=761.76ms p(90)=531.56ms p(95)=565.6ms 
       { expected_response:true }...: avg=381.05ms min=205.64ms med=360.35ms max=761.76ms p(90)=531.56ms p(95)=565.6ms 
     http_req_failed................: 0.00% ✓ 0        ✗ 100
     http_req_receiving.............: avg=111.11µs min=55µs     med=72µs     max=2.04ms   p(90)=123.2µs  p(95)=171.24µs
     http_req_sending...............: avg=32.86µs  min=16µs     med=25µs     max=462µs    p(90)=35µs     p(95)=50.05µs 
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=380.9ms  min=205.5ms  med=359.82ms max=759.7ms  p(90)=531.48ms p(95)=565.5ms 
     http_reqs......................: 100   2.622438/s
     iteration_duration.............: avg=381.29ms min=205.85ms med=360.51ms max=761.91ms p(90)=531.72ms p(95)=565.77ms
     iterations.....................: 100   2.622438/s
     vus............................: 1     min=1      max=1
     vus_max........................: 1     min=1      max=1


running (00m38.1s), 0/1 VUs, 100 complete and 0 interrupted iterations
default ✓ [ 100% ] 1 VUs  00m38.1s/10m0s  100/100 shared iters

Incoming messages from WhatsApp flow with same visitor on every request.

          /\      |‾‾| /‾‾/   /‾‾/   
     /\  /  \     |  |/  /   /  /    
    /  \/    \    |     (   /   ‾‾\  
   /          \   |  |\  \ |  (‾)  | 
  / __________ \  |__| \__\ \_____/ .io

     execution: local
        script: src/incoming-message.js
        output: -

     scenarios: (100.00%) 1 scenario, 1 max VUs, 10m30s max duration (incl. graceful stop):
              * default: 100 iterations shared among 1 VUs (maxDuration: 10m0s, gracefulStop: 30s)


     data_received..................: 76 kB 4.6 kB/s
     data_sent......................: 46 kB 2.8 kB/s
     http_req_blocked...............: avg=31.16µs  min=2µs     med=3µs      max=2.76ms   p(90)=5µs      p(95)=8.04µs  
     http_req_connecting............: avg=2.46µs   min=0s      med=0s       max=246µs    p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=164.44ms min=83.46ms med=146.14ms max=446.67ms p(90)=223.67ms p(95)=252.44ms
       { expected_response:true }...: avg=164.44ms min=83.46ms med=146.14ms max=446.67ms p(90)=223.67ms p(95)=252.44ms
     http_req_failed................: 0.00% ✓ 0        ✗ 100
     http_req_receiving.............: avg=119.44µs min=51µs    med=71µs     max=2.95ms   p(90)=114.6µs  p(95)=160.64µs
     http_req_sending...............: avg=27.39µs  min=16µs    med=24µs     max=120µs    p(90)=37µs     p(95)=42.05µs 
     http_req_tls_handshaking.......: avg=0s       min=0s      med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=164.3ms  min=83.38ms med=146.05ms max=446.56ms p(90)=223.58ms p(95)=252.31ms
     http_reqs......................: 100   6.069645/s
     iteration_duration.............: avg=164.74ms min=83.6ms  med=146.3ms  max=446.9ms  p(90)=223.83ms p(95)=252.66ms
     iterations.....................: 100   6.069645/s
     vus............................: 1     min=1      max=1
     vus_max........................: 1     min=1      max=1


running (00m16.5s), 0/1 VUs, 100 complete and 0 interrupted iterations
default ✓ [================] 1 VUs  00m16.5s/10m0s  100/100 shared iters

Copy link
Contributor

dionisio-bot bot commented Jun 18, 2024

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is targeting the wrong base branch. It should target 6.11.0, but it targets 6.10.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented Jun 18, 2024

⚠️ No Changeset found

Latest commit: 39ff1dd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

codecov bot commented Jun 18, 2024

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 56.65%. Comparing base (b5866a1) to head (39ff1dd).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #32620      +/-   ##
===========================================
- Coverage    56.75%   56.65%   -0.11%     
===========================================
  Files         2498     2494       -4     
  Lines        55390    55387       -3     
  Branches     11457    11457              
===========================================
- Hits         31439    31378      -61     
- Misses       21243    21308      +65     
+ Partials      2708     2701       -7     
Flag Coverage Δ
e2e 56.46% <ø> (-0.11%) ⬇️
unit 71.88% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@CLAassistant
Copy link

CLAassistant commented Jun 26, 2024

CLA assistant check
All committers have signed the CLA.

@ricardogarim ricardogarim force-pushed the refactor/defineVisitor branch 2 times, most recently from 9dbd8e8 to c869396 Compare June 26, 2024 21:05
@ggazzo ggazzo added this to the 6.11 milestone Jun 26, 2024
@sampaiodiego sampaiodiego merged commit 7a57f34 into develop Jun 27, 2024
48 checks passed
@sampaiodiego sampaiodiego deleted the refactor/defineVisitor branch June 27, 2024 21:46
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.

None yet

4 participants