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

Add request ID in log #1604

Merged
merged 15 commits into from
Jul 7, 2020
Merged

Add request ID in log #1604

merged 15 commits into from
Jul 7, 2020

Conversation

Aschen
Copy link
Contributor

@Aschen Aschen commented Apr 23, 2020

What does this PR do ?

Use the new AsyncLocalStorage of Node.js 14 to share a context between all the asynchronous calls for processing an API request.

If Kuzzle run with Node.js 12, a no-op class is used instead.

The performance overhead is 8%

---- Log with AsyncLocalStorage Log classic difference
req/s 2613 2842 8%
Complete benchmark details
# server:now
  /**
   * @returns {Promise<Object>}
   */
  async now () {
    this.kuzzle.log.info('ASL FTW');

    return { now: Date.now() };
  }

Benchmark with wrk and NODE_ENV=production

# With AsyncLocalStorage

$ wrk -t12 -c300 -d30s http://localhost:7512/_now
Running 30s test @ http://localhost:7512/_now
  12 threads and 300 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   119.06ms   53.96ms 659.90ms   94.74%
    Req/Sec   230.42     51.60   300.00     87.08%
  78675 requests in 30.10s, 46.67MB read
Requests/sec:   2613.86
Transfer/sec:      1.55MB

# Without AsyncLocalStorage

$ wrk -t12 -c300 -d30s http://localhost:7512/_now
Running 30s test @ http://localhost:7512/_now
  12 threads and 300 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   105.60ms   30.22ms 301.36ms   85.67%
    Req/Sec   244.67     84.32   505.00     74.93%
  85509 requests in 30.08s, 50.72MB read
Requests/sec:   2842.67
Transfer/sec:      1.69MB

How should this be manually tested?

  • Step 1 :
  • Step 2 :
  • Step 3 :
    ...

Other changes

Boyscout

@Aschen Aschen marked this pull request as draft April 23, 2020 12:32
@sonarcloud
Copy link

sonarcloud bot commented Apr 26, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.1% 0.1% Duplication

@codecov
Copy link

codecov bot commented Apr 26, 2020

Codecov Report

Merging #1604 into 2-dev will decrease coverage by 0.00%.
The diff coverage is 90.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##            2-dev    #1604      +/-   ##
==========================================
- Coverage   93.52%   93.51%   -0.01%     
==========================================
  Files         113      114       +1     
  Lines        7197     7221      +24     
==========================================
+ Hits         6731     6753      +22     
- Misses        466      468       +2     
Impacted Files Coverage Δ
lib/kuzzle/log.js 69.23% <42.85%> (-8.55%) ⬇️
lib/api/funnel.js 96.64% <100.00%> (+0.02%) ⬆️
lib/kuzzle/kuzzle.js 95.78% <100.00%> (+0.09%) ⬆️
lib/util/asyncStore.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0d91591...935ecdf. Read the comment docs.

@Aschen Aschen marked this pull request as ready for review July 1, 2020 04:51
@Aschen Aschen requested a review from Leodau July 1, 2020 04:52
@Aschen Aschen force-pushed the experiment-async-local-storage branch from f5721eb to 50999ad Compare July 1, 2020 04:55
@sonarcloud
Copy link

sonarcloud bot commented Jul 1, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.1% 0.1% Duplication

@Aschen
Copy link
Contributor Author

Aschen commented Jul 1, 2020

@Leodau You may want to review this PR again

@Leodau Leodau merged commit 1f584a2 into 2-dev Jul 7, 2020
@Leodau Leodau deleted the experiment-async-local-storage branch July 7, 2020 07:45
This was referenced Jul 7, 2020
scottinet added a commit that referenced this pull request Jul 8, 2020
# [2.3.2](https://github.com/kuzzleio/kuzzle/releases/tag/2.3.2) (2020-07-07)


#### Bug fixes

- [ [#1701](#1701) ] Fix: description should not be required for plugin custom errors   ([scottinet](https://github.com/scottinet))
- [ [#1695](#1695) ] Fix dump generation   ([Aschen](https://github.com/Aschen))
- [ [#1698](#1698) ] Fix generic events on search queries   ([scottinet](https://github.com/scottinet))

#### Enhancements

- [ [#1702](#1702) ] Update search indexes only when the "dynamic" setting changes from false to true/strict   ([Aschen](https://github.com/Aschen))
- [ [#1630](#1630) ] Loose coupling: security module   ([scottinet](https://github.com/scottinet))
- [ [#1604](#1604) ] Add request ID in log   ([Aschen](https://github.com/Aschen))
---
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