Fix issue #1627 - stats usage #1629

rg2011 · 2024-08-02T11:37:19Z

Fixes issue #1627 - comment #1627 (comment)

Removes push stats: local stats, periodic stat refresh, and documentation for the config.stats section.
Adds support for pull stats by exposing an openmetrics endpoint in the /metrics path of the northbound API.
Documents the /metrics endpoint.

The /metrics path is a convention for any openmetrics exporter.

fgalan · 2024-08-02T11:39:09Z

CHANGES_NEXT_RELEASE

@@ -1,3 +1,5 @@
+- Add: openmetrics-compatible `/metrics` endpoint in nortbound API
+- Deprecate: push-based stats


Maybe we should go beyond deprecation and remove the functionality (given that nobody is using it and, moreover, it probably doesn't work).

I'm good with both, @AlvaroVega what do you think?

I'm agree on remove previous functionality.

CHANGES_NEXT_RELEASE

Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>

fgalan · 2024-08-02T12:25:07Z

Take care of lint:

/home/runner/work/iotagent-node-lib/iotagent-node-lib/lib/fiware-iotagent-lib.js
Error:   51:5   error  'logActions' is not defined  no-undef
Error:   65:9   error  'logActions' is not defined  no-undef
Error:   67:18  error  'logActions' is not defined  no-undef

/home/runner/work/iotagent-node-lib/iotagent-node-lib/lib/services/stats/statsRegistry.js
Error:   228:28  error  Expected '!==' and instead saw '!='          eqeqeq
Error:   229:22  error  'context' is not defined                     no-undef
Error:   236:36  error  Expected '!==' and instead saw '!='          eqeqeq
Error:   237:30  error  'context' is not defined                     no-undef
Error:   246:26  error  'context' is not defined                     no-undef
Error:   250:21  error  The array literal notation [] is preferable  no-array-constructor

rg2011 · 2024-08-02T13:01:11Z

Take care of lint:

fixed.

fgalan · 2024-08-05T07:44:48Z

lib/model/dbConn.js

-        (currentConfig.deviceRegistry && currentConfig.deviceRegistry.type === 'mongodb') ||
-        (currentConfig.stats && currentConfig.stats.persistence === true)
-    ) {
+    if (currentConfig.deviceRegistry && currentConfig.deviceRegistry.type === 'mongodb') {


https://github.com/telefonicaid/iotagent-node-lib/blob/master/doc/admin.md#stats section in documentation should be removed? Or it is still used?

it has been removed, no longer used.

Ok. I missed that... NTC

CHANGES_NEXT_RELEASE

Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>

AlvaroVega · 2024-08-05T07:51:53Z

lib/services/ngsi/ngsiService.js

@@ -67,7 +68,8 @@ function init() {
 * @param {String} token            User token to identify against the PEP Proxies (optional).
 */
 function sendUpdateValue(entityName, attributes, typeInformation, token, callback) {
-    entityHandler.sendUpdateValue(entityName, attributes, typeInformation, token, callback);
+    const newCallback = statsRegistry.withStats('updateEntityRequestsOk', 'updateEntityRequestsError', callback);


only here and raise/release alarm are feeding statsRegistry at this moment. It should be added also for create/delete groups/devices and so on ?

I only added metrics for things I want to alert on our monitoring systems. Currently, I want to create alerts:

If the number of failed updates increases much above average,

If the number of successful updates falls much below average,

if the number of alarms RAISE is higher than the number of alarms RELEASE.

So I only added metrics for those three use cases. It might be better to add metrics on demand, when some use case requires them.

fgalan

LGTM

Passing the ball to @AlvaroVega for extra review and LGTM before merging

AlvaroVega

LGTM

rg2011 added 30 commits July 26, 2024 10:58

added stats env variables

816753d

Merge branch 'master' into fix/issue-1627

f521e22

Update CNR

14bed96

Update admin.md

67674d7

Update admin.md

679b7d1

Update admin.md

0bb40cd

added prometheus exporter

04bacb1

fix include path

1664ca3

fix paths

ad0cab2

fix stats collection

539a6f0

unconditionally publish global stats

89a74e7

fixed reference to hasOwnProperty

908d6f3

improve metrics format

22dfddb

fix some typos and add all global stats

b7bad16

remove superflouos comments

390a0fc

remove noise from the PR

34b47e7

add support for openmetrics

e549c8c

improve processing of accepts header

5142a8b

Improve content-type negotation for openmetrics

3cd0463

improve header negotiation

600791f

optimize not getting stats if header is wrong

90bf909

improved negotation

e2b07ff

fix typo

863ab5e

fix accept headers

d0faa5d

improve negotiation

50f904e

Merge branch 'master' into fix/issue-1627

16da95b

Merge branch 'fix/issue-1627' into fix/prometheus

32b70f8

improve comments

836d5e7

added doc

31f1071

updated documetation

2d43858

rg2011 added 3 commits August 2, 2024 10:55

remove unneeded changes

f7eedbe

reallocate metrics code

1b44fe3

improve docs

14cc8d6

rg2011 mentioned this pull request Aug 2, 2024

[DO NOT MERGE]Add environment variables for stats config (#1627) #1628

Closed

fgalan reviewed Aug 2, 2024

View reviewed changes

CHANGES_NEXT_RELEASE Outdated Show resolved Hide resolved

Update CHANGES_NEXT_RELEASE

683121f

Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>

Remove push-based stats

b770380

rg2011 requested review from AlvaroVega and fgalan August 5, 2024 07:14

fgalan reviewed Aug 5, 2024

View reviewed changes

CHANGES_NEXT_RELEASE Outdated Show resolved Hide resolved

Update CHANGES_NEXT_RELEASE

2c9772d

Co-authored-by: Fermín Galán Márquez <fgalan@users.noreply.github.com>

AlvaroVega reviewed Aug 5, 2024

View reviewed changes

fgalan approved these changes Aug 5, 2024

View reviewed changes

fgalan mentioned this pull request Aug 5, 2024

Add environment variable for config.stats.interval #1627

Closed

AlvaroVega approved these changes Aug 7, 2024

View reviewed changes

AlvaroVega merged commit 04bb324 into telefonicaid:master Aug 7, 2024
7 of 8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue #1627 - stats usage #1629

Fix issue #1627 - stats usage #1629

rg2011 commented Aug 2, 2024 •

edited

Loading

fgalan Aug 2, 2024

rg2011 Aug 2, 2024

AlvaroVega Aug 2, 2024

fgalan commented Aug 2, 2024

rg2011 commented Aug 2, 2024

fgalan Aug 5, 2024

rg2011 Aug 5, 2024

fgalan Aug 5, 2024

AlvaroVega Aug 5, 2024

rg2011 Aug 5, 2024 •

edited

Loading

fgalan left a comment

AlvaroVega left a comment

		@@ -1,3 +1,5 @@
		- Add: openmetrics-compatible `/metrics` endpoint in nortbound API
		- Deprecate: push-based stats

Fix issue #1627 - stats usage #1629

Fix issue #1627 - stats usage #1629

Conversation

rg2011 commented Aug 2, 2024 • edited Loading

fgalan Aug 2, 2024

Choose a reason for hiding this comment

rg2011 Aug 2, 2024

Choose a reason for hiding this comment

AlvaroVega Aug 2, 2024

Choose a reason for hiding this comment

fgalan commented Aug 2, 2024

rg2011 commented Aug 2, 2024

fgalan Aug 5, 2024

Choose a reason for hiding this comment

rg2011 Aug 5, 2024

Choose a reason for hiding this comment

fgalan Aug 5, 2024

Choose a reason for hiding this comment

AlvaroVega Aug 5, 2024

Choose a reason for hiding this comment

rg2011 Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

fgalan left a comment

Choose a reason for hiding this comment

AlvaroVega left a comment

Choose a reason for hiding this comment

rg2011 commented Aug 2, 2024 •

edited

Loading

rg2011 Aug 5, 2024 •

edited

Loading