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

Switch to gomodules and upgrade dependencies #70

Closed
wants to merge 6 commits into from
Closed

Conversation

roobre
Copy link
Contributor

@roobre roobre commented Jul 1, 2021

In this PR we get rid of the vendor folder and govendor tool, and switch to gomodules instead.

Additionally, the versions of most of the dependencies have been upgraded.

Notably:

  • github.com/docker/docker was updated to the latest version, which required minor changes on how the client is created
  • github.com/containerd/cgroups was upgraded to v1.0.1, which moved the Metrics object to a subpackage, requiring minimal changes as well.

No other breaking changes have been identified.

@roobre roobre force-pushed the go-modules branch 4 times, most recently from 9fed617 to 03bc03f Compare July 1, 2021 13:35
@roobre
Copy link
Contributor Author

roobre commented Jul 2, 2021

Here's a diff from my machine, comparing the output of this pr with the previous version, 1.4.x:

--- <unnamed>
+++ <unnamed>
@@ -1,3 +1,4 @@
+[WARN] computing Storage Driver stats: only devicemapper is supported
 [ERR] error fetching metrics for container b2a3b90a11309aaf0f28290b384712b98821f858586107cf56879d525361f525: cgroups: cgroup deleted
 [ERR] error fetching metrics for container 10f6561f385263d59f37eb6b377aa4579daf35f369eb0283bd88731244c432ed: failed to open file: /proc/0/cgroup, while detecting cgroup paths error: open /proc/0/cgroup: no such file or directory
 [ERR] error fetching metrics for container caacb9883a6ab95fe15fa11a2a1d3b797c4e3444f8eab167c9d717521ee1b826: cgroups: cgroup deleted
@@ -11,7 +12,7 @@
 {
   "name":"com.newrelic.docker",
   "protocol_version":"3",
-  "integration_version":"1.4.3",
+  "integration_version":"v1.6.0-SNAPSHOT-03bc03f",
   "data":[
     {
       "entity":{
@@ -28,6 +29,7 @@
           "cpuKernelPercent":0,
           "cpuLimitCores":0,
           "cpuPercent":0,
+          "cpuShares":0,
           "cpuThrottlePeriods":0,
           "cpuThrottleTimeMs":0,
           "cpuUsedCores":0,
@@ -63,7 +65,10 @@
           "label.org.opencontainers.image.url":"https://github.com/mbround18/valheim-docker",
           "label.org.opencontainers.image.version":"1.4.2",
           "memoryCacheBytes":0,
-          "memoryResidentSizeBytes":0,
+          "memoryKernelUsageBytes":0,
+          "memoryResidentSizeBytes":0,
+          "memorySwapOnlyUsageBytes":0,
+          "memorySwapUsageBytes":0,
           "memoryUsageBytes":0,
           "name":"valheim_valheim_1",
           "networkRxBytes":0,
@@ -150,6 +155,7 @@
           "cpuKernelPercent":0,
           "cpuLimitCores":0,
           "cpuPercent":0,
+          "cpuShares":0,
           "cpuThrottlePeriods":0,
           "cpuThrottleTimeMs":0,
           "cpuUsedCores":0,
@@ -178,7 +184,10 @@
           "label.com.docker.compose.version":"1.29.1",
           "label.maintainer":"thelamer",
           "memoryCacheBytes":0,
-          "memoryResidentSizeBytes":0,
+          "memoryKernelUsageBytes":0,
+          "memoryResidentSizeBytes":0,
+          "memorySwapOnlyUsageBytes":0,
+          "memorySwapUsageBytes":0,
           "memoryUsageBytes":0,
           "name":"jackett",
           "networkRxBytes":0,
@@ -227,6 +236,7 @@
           "cpuKernelPercent":0,
           "cpuLimitCores":0,
           "cpuPercent":0,
+          "cpuShares":0,
           "cpuThrottlePeriods":0,
           "cpuThrottleTimeMs":0,
           "cpuUsedCores":0,
@@ -255,7 +265,10 @@
           "label.com.docker.compose.version":"1.29.1",
           "label.maintainer":"aptalca",
           "memoryCacheBytes":0,
-          "memoryResidentSizeBytes":0,
+          "memoryKernelUsageBytes":0,
+          "memoryResidentSizeBytes":0,
+          "memorySwapOnlyUsageBytes":0,
+          "memorySwapUsageBytes":0,
           "memoryUsageBytes":0,
           "name":"sonarr",
           "networkRxBytes":0,
@@ -304,6 +317,7 @@
           "cpuKernelPercent":0,
           "cpuLimitCores":0,
           "cpuPercent":0,
+          "cpuShares":0,
           "cpuThrottlePeriods":0,
           "cpuThrottleTimeMs":0,
           "cpuUsedCores":0,
@@ -332,7 +346,10 @@
           "label.com.docker.compose.version":"1.29.1",
           "label.maintainer":"alex-phillips, homerr",
           "memoryCacheBytes":0,
-          "memoryResidentSizeBytes":0,
+          "memoryKernelUsageBytes":0,
+          "memoryResidentSizeBytes":0,
+          "memorySwapOnlyUsageBytes":0,
+          "memorySwapUsageBytes":0,
           "memoryUsageBytes":0,
           "name":"grocy",
           "networkRxBytes":0,
@@ -381,6 +398,7 @@
           "cpuKernelPercent":0,
           "cpuLimitCores":0,
           "cpuPercent":0,
+          "cpuShares":0,
           "cpuThrottlePeriods":0,
           "cpuThrottleTimeMs":0,
           "cpuUsedCores":0,
@@ -409,7 +427,10 @@
           "label.com.docker.compose.version":"1.29.1",
           "label.maintainer":"sparklyballs,aptalca",
           "memoryCacheBytes":0,
-          "memoryResidentSizeBytes":0,
+          "memoryKernelUsageBytes":0,
+          "memoryResidentSizeBytes":0,
+          "memorySwapOnlyUsageBytes":0,
+          "memorySwapUsageBytes":0,
           "memoryUsageBytes":0,
           "name":"medusa",
           "networkRxBytes":0,
@@ -458,6 +479,7 @@
           "cpuKernelPercent":0,
           "cpuLimitCores":0,
           "cpuPercent":0,
+          "cpuShares":0,
           "cpuThrottlePeriods":0,
           "cpuThrottleTimeMs":0,
           "cpuUsedCores":0,
@@ -492,7 +514,10 @@
           "label.org.opencontainers.image.url":"https://github.com/FlareSolverr/FlareSolverr",
           "label.org.opencontainers.image.version":"v1.2.5",
           "memoryCacheBytes":0,
-          "memoryResidentSizeBytes":0,
+          "memoryKernelUsageBytes":0,
+          "memoryResidentSizeBytes":0,
+          "memorySwapOnlyUsageBytes":0,
+          "memorySwapUsageBytes":0,
           "memoryUsageBytes":0,
           "name":"flaresolverr",
           "networkRxBytes":0,
@@ -541,6 +566,7 @@
           "cpuKernelPercent":0,
           "cpuLimitCores":0,
           "cpuPercent":0,
+          "cpuShares":0,
           "cpuThrottlePeriods":0,
           "cpuThrottleTimeMs":0,
           "cpuUsedCores":0,
@@ -567,7 +593,10 @@
           "label.com.docker.compose.service":"owncast",
           "label.com.docker.compose.version":"1.29.1",
           "memoryCacheBytes":0,
-          "memoryResidentSizeBytes":0,
+          "memoryKernelUsageBytes":0,
+          "memoryResidentSizeBytes":0,
+          "memorySwapOnlyUsageBytes":0,
+          "memorySwapUsageBytes":0,
           "memoryUsageBytes":0,
           "name":"owncast",
           "networkRxBytes":0,
@@ -616,6 +645,7 @@
           "cpuKernelPercent":0,
           "cpuLimitCores":0,
           "cpuPercent":0,
+          "cpuShares":0,
           "cpuThrottlePeriods":0,
           "cpuThrottleTimeMs":0,
           "cpuUsedCores":0,
@@ -644,7 +674,10 @@
           "label.com.docker.compose.version":"1.29.1",
           "label.maintainer":"thelamer",
           "memoryCacheBytes":0,
-          "memoryResidentSizeBytes":0,
+          "memoryKernelUsageBytes":0,
+          "memoryResidentSizeBytes":0,
+          "memorySwapOnlyUsageBytes":0,
+          "memorySwapUsageBytes":0,
           "memoryUsageBytes":0,
           "name":"radarr",
           "networkRxBytes":0,
@@ -693,6 +726,7 @@
           "cpuKernelPercent":0,
           "cpuLimitCores":0,
           "cpuPercent":0,
+          "cpuShares":0,
           "cpuThrottlePeriods":0,
           "cpuThrottleTimeMs":0,
           "cpuUsedCores":0,
@@ -721,7 +755,10 @@
           "label.com.docker.compose.version":"1.29.1",
           "label.maintainer":"thelamer",
           "memoryCacheBytes":0,
-          "memoryResidentSizeBytes":0,
+          "memoryKernelUsageBytes":0,
+          "memoryResidentSizeBytes":0,
+          "memorySwapOnlyUsageBytes":0,
+          "memorySwapUsageBytes":0,
           "memoryUsageBytes":0,
           "name":"jellyfin",
           "networkRxBytes":0,

@alvarocabanas alvarocabanas self-requested a review July 15, 2021 10:16
@invidian
Copy link

/assign

@paologallinaharbur
Copy link
Member

paologallinaharbur commented Jul 15, 2021

Checking the diff It seems that new metrics are appearing, should we updated docs and infra-specs? @roobre

EDIT: they are already in the docs, therefore I guess we were already collecting them

bazil.org/fuse v0.0.0-20160811212531-371fbbdaa898 h1:SC+c6A1qTFstO9qmB86mPV2IpYme/2ZoEQ0hrP+wo+Q=
bazil.org/fuse v0.0.0-20160811212531-371fbbdaa898/go.mod h1:Xbm+BRKSBEpa4q4hTSxohYNQpsxXPbPry4JJWOB3LB8=
cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Copy link
Member

Choose a reason for hiding this comment

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

Running go mod tidy currently changes this file

Copy link
Member

@paologallinaharbur paologallinaharbur left a comment

Choose a reason for hiding this comment

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

Overall looks good, only few doubts. It is difficult to understand if bumping that version could break something without investigating

src/nri/sampler.go Show resolved Hide resolved
src/docker.go Show resolved Hide resolved
Copy link

@invidian invidian left a comment

Choose a reason for hiding this comment

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

LGTM

@roobre
Copy link
Contributor Author

roobre commented Jul 15, 2021

@paologallinaharbur Yes, I did the diff not with the latest released version, but with one version earlier. My bad :(

@paologallinaharbur
Copy link
Member

@roobre Also this one is becoming stale

@roobre
Copy link
Contributor Author

roobre commented Aug 25, 2021

You're right, I wanted to do some final testing to ensure everything works as expected but in the end I wasn't able to :(

I'll try to do them today

@paologallinaharbur
Copy link
Member

stale, closing for now. Addressing cGroups 2 we are going to iterate over this again

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.

4 participants