n1ru4l / envelop

Envelop is a lightweight library allowing developers to easily develop, share, collaborate and extend their GraphQL execution layer. Envelop is the missing GraphQL plugin system.
https://envelop.dev
MIT License
771 stars 119 forks source link

[plugins/prometheus] Fix registry metrics duplication #2217

Closed EmrysMyrddin closed 2 months ago

EmrysMyrddin commented 2 months ago

Description

Is this plugin is initialized multiple times with the same registry (which can happen in the context of Mesh when polling is enabled for example).

This PR aims to fix this.

Type of change

Further comments

Current solution rely on caching Histograms with a Map to reuse them instead of creating them each time.

But this has a drawback: if the configuration of those Histograms changes, this changement will be silently ignored and the old configuration will be kept in use.

changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: 5841b485801f79abc658ebd6102aaa53c6bf5cef

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ------------------- | ----- | | @envelop/prometheus | Major |

Not sure what this means? Click here to learn what changesets are.

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

github-actions[bot] commented 2 months ago
### 💻 Website Preview The latest changes are available as preview in: [https://587986fa.envelop.pages.dev](https://587986fa.envelop.pages.dev)
github-actions[bot] commented 2 months ago

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets): Package Version Info
@envelop/opentelemetry 6.3.1-alpha-20240507125432-5841b485 npm ↗︎ unpkg ↗︎
@envelop/prometheus 10.0.0-alpha-20240507125432-5841b485 npm ↗︎ unpkg ↗︎
theguild-bot commented 2 months ago

✅ Benchmark Results

     ✓ no_errors
     ✓ expected_result

     checks.............................................: 100.00% ✓ 880594      ✗ 0     
     ✓ { mode:envelop-cache-and-no-internal-tracing }...: 100.00% ✓ 214734      ✗ 0     
     ✓ { mode:envelop-cache-jit }.......................: 100.00% ✓ 338712      ✗ 0     
     ✓ { mode:envelop-just-cache }......................: 100.00% ✓ 211822      ✗ 0     
     ✓ { mode:graphql-js }..............................: 100.00% ✓ 115326      ✗ 0     
     data_received......................................: 3.4 GB  28 MB/s
     data_sent..........................................: 192 MB  1.6 MB/s
     envelop_init.......................................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-cache-jit }.......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-just-cache }......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:graphql-js }..............................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     envelop_total......................................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-cache-jit }.......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-just-cache }......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:graphql-js }..............................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     event_loop_lag.....................................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-cache-and-no-internal-tracing }...: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-cache-jit }.......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-just-cache }......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:graphql-js }..............................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     graphql_context....................................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-cache-jit }.......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-just-cache }......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:graphql-js }..............................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     graphql_execute....................................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-cache-jit }.......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-just-cache }......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:graphql-js }..............................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     graphql_parse......................................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-cache-jit }.......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-just-cache }......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:graphql-js }..............................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     graphql_validate...................................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-cache-jit }.......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:envelop-just-cache }......................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     ✓ { mode:graphql-js }..............................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_blocked...................................: avg=1.83µs  min=722ns    med=1.66µs  max=8.22ms  p(90)=2.22µs  p(95)=2.38µs 
     http_req_connecting................................: avg=21ns    min=0s       med=0s      max=1.45ms  p(90)=0s      p(95)=0s     
     http_req_duration..................................: avg=2.45ms  min=150.6µs  med=2.12ms  max=81.81ms p(90)=4.46ms  p(95)=4.92ms 
       { expected_response:true }.......................: avg=2.45ms  min=150.6µs  med=2.12ms  max=81.81ms p(90)=4.46ms  p(95)=4.92ms 
     ✓ { mode:envelop-cache-and-no-internal-tracing }...: avg=2.52ms  min=327.02µs med=2.21ms  max=17.66ms p(90)=4.4ms   p(95)=4.79ms 
     ✓ { mode:envelop-cache-jit }.......................: avg=1.48ms  min=150.6µs  med=1.19ms  max=15.58ms p(90)=2.43ms  p(95)=2.59ms 
     ✓ { mode:envelop-just-cache }......................: avg=2.55ms  min=370.01µs med=2.23ms  max=34.31ms p(90)=4.43ms  p(95)=4.83ms 
     ✓ { mode:graphql-js }..............................: avg=4.95ms  min=698.55µs med=4.17ms  max=81.81ms p(90)=8.3ms   p(95)=8.92ms 
     http_req_failed....................................: 0.00%   ✓ 0           ✗ 440297
     http_req_receiving.................................: avg=30.47µs min=12.11µs  med=27.41µs max=12.51ms p(90)=40.64µs p(95)=45.19µs
     http_req_sending...................................: avg=11.62µs min=4.47µs   med=9.91µs  max=9.17ms  p(90)=13.77µs p(95)=17.4µs 
     http_req_tls_handshaking...........................: avg=0s      min=0s       med=0s      max=0s      p(90)=0s      p(95)=0s     
     http_req_waiting...................................: avg=2.4ms   min=124.13µs med=2.07ms  max=81.76ms p(90)=4.42ms  p(95)=4.87ms 
     http_reqs..........................................: 440297  3669.080993/s
     iteration_duration.................................: avg=2.71ms  min=358.18µs med=2.38ms  max=82.33ms p(90)=4.72ms  p(95)=5.21ms 
     iterations.........................................: 440297  3669.080993/s
     vus................................................: 10      min=10        max=10  
     vus_max............................................: 20      min=20        max=20  
dotansimha commented 2 months ago

this changement will be silently ignored and the old configuration will be kept in use. By "silent", you mean without any warning? should we at least warn or print something to the log? how silent it is?

EmrysMyrddin commented 2 months ago

@dotansimha We don't actually have any means to know if the configuration have changed or not. Even if we keep track of the previous configuration somehow and check fo the deep equality with the new config, it will most likely be always different, because of the fillLabels being a function, which will always be uniq even if it has the exact same implementation.

We could add a warning if we detect that a metrics was already registered with this name, but then will have this warning every time we create a new Mesh instance when polling is enable.

Do you think I should add a warning with an option to disable it explicitly ?