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.

Sign up for GitHub

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

Jump to bottom

optl: Histogram to Counter #1467

Merged
merged 1 commit into from
Jun 26, 2023
Merged

optl: Histogram to Counter #1467

merged 1 commit into from
Jun 26, 2023

Conversation

LionelJouin
Copy link
Member

@LionelJouin LionelJouin commented Jun 10, 2023 •
edited
Loading

Description

For the kind of metrics currently colllected in NSM (rx/tx packets/bytes, drops...), Having counters is more appropriate than using histogram.

Probably I will need to save the previous value and apply the difference since counter only have a "Add" function.

I also updated the open telemetry dependency to v1.16.0. Updating to golang 1.20 is required for this version of OpenTelemetry.

Issue link

#1464
networkservicemesh/deployments-k8s#9216

How Has This Been Tested?

  • Added unit testing to cover
  • Tested manually
  • Tested by integration testing
  • Have not tested

Types of changes

  • Bug fix
  • New functionality
  • Documentation
  • Refactoring
  • CI

@LionelJouin LionelJouin marked this pull request as ready for review June 21, 2023 13:57
@LionelJouin LionelJouin mentioned this pull request Jun 21, 2023
Closed
9 tasks
@LionelJouin LionelJouin added the enhancement New feature or request label Jun 21, 2023
@codecov
Copy link

codecov bot commented Jun 21, 2023 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (main@63043b2). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1467   +/-   ##
=======================================
  Coverage        ?   71.63%           
=======================================
  Files           ?      241           
  Lines           ?    10720           
  Branches        ?        0           
=======================================
  Hits            ?     7679           
  Misses          ?     2570           
  Partials        ?      471

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

denis-tingaikin
denis-tingaikin reviewed Jun 22, 2023
View reviewed changes
Copy link
Member

@denis-tingaikin denis-tingaikin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Could we update go in the separate PR?

Also, something is happend with build, could you have a look at job result?

@glazychev-art
Copy link
Contributor

I think first we need to update .github before, since we are using reusable workflow, and there is v1.18.
(here for example - https://github.com/networkservicemesh/.github/blob/main/.github/workflows/build-and-test.yaml#L22)

@LionelJouin
Copy link
Member Author

networkservicemesh/.github#32

@denis-tingaikin
Copy link
Member

@LionelJouin Merged

@LionelJouin
Copy link
Member Author

Thank you @denis-tingaikin, how does it work? I can still see this go version error: https://github.com/networkservicemesh/sdk/actions/runs/5343501614/jobs/9759105840?pr=1467

@denis-tingaikin
Copy link
Member

@LionelJouin My guess that you need simply force push the branch

@LionelJouin
optl: Histogram to Counter
790ae0f 
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: Lionel Jouin <lionel.jouin@est.tech>
@LionelJouin
Copy link
Member Author

Still got errors, I tried go test -race ./... and go test -coverprofile=coverage-ubuntu-latest.txt -covermode=atomic -race ./... locally and had no problem.

@denis-tingaikin
Copy link
Member

Could you add option -count 1 ?

go test -race ./... -count 1

Seems to me we have issues in the sdk.
Anyway lets try to quickly fix the problems if its actually is not quick we'll continue on our side ;)

@LionelJouin
Copy link
Member Author

Thank you, just tried, and same, all tests passed with no problem.

@LionelJouin
Copy link
Member Author

@denis-tingaikin did you changed anything? Now only macos is failing (I don't have macos)

@denis-tingaikin
Copy link
Member

@LionelJouin I've restarted job a few times and seems like everything is passed. I think we should merge it and stabilize CI in separate PR @glazychev-art thoughts?

glazychev-art
glazychev-art approved these changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@glazychev-art glazychev-art left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Most likely, the instabilities are not related to this PR.

@denis-tingaikin denis-tingaikin merged commit e977214 into networkservicemesh:main Jun 26, 2023
nsmbot pushed a commit to networkservicemesh/cmd-ipam-vl3 that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
db13bd4 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-cluster-info-k8s that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
c799d99 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-vfio that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
6a30d54 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-admission-webhook-k8s that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
d9f5abb 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-registry-memory that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
a007998 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
@nsmbot nsmbot mentioned this pull request Jun 26, 2023
Merged
nsmbot pushed a commit to networkservicemesh/sdk-k8s that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
162c244 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
This was referenced Jun 26, 2023
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
nsmbot pushed a commit to networkservicemesh/sdk-kernel that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
1bbca03 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-map-ip-k8s that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
0dba974 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-admission-webhook-k8s that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
62d7cea 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-registry-proxy-dns that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
b0cb2ac 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-cluster-info-k8s that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
b4cee5e 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
e3e242f 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-ipam-vl3 that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
f9a0148 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-registry-memory that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
4085606 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-remote-vlan that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
53b8f95 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr-proxy that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
e7fee50 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-vfio that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
91bee4e 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsc-init that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
690aa16 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-k8s that referenced this pull request Jun 26, 2023
Update go.mod and go.sum to latest version from networkservicemesh/sd…
65ccdda 
…k@main

PR link: networkservicemesh/sdk#1467

Commit: e977214
Author: Lionel Jouin
Date: 2023-06-26 21:40:14 +0200
Message:
  - optl: Histogram to Counter (#1467)
For the kind of metrics currently colllected in NSM (rx/tx
packets/bytes, drops...), Having counters is more appropriate than using
histogram.
Update to golang 1.20

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
This was referenced Jun 26, 2023
Merged
Merged
@LionelJouin LionelJouin mentioned this pull request Jun 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@denis-tingaikin denis-tingaikin denis-tingaikin approved these changes

@glazychev-art glazychev-art glazychev-art approved these changes

@NikitaSkrynnik NikitaSkrynnik Awaiting requested review from NikitaSkrynnik

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LionelJouin @glazychev-art @denis-tingaikin