Migrate to SDK v3 #160

roobre · 2021-07-16T12:07:27Z

This PR updates the integration SDK to version 3.

Closes #145

roobre · 2021-07-16T12:46:00Z

Linting errors related to goimports will be tackled later since they encompass even more files than those that this PR touches already

roobre · 2021-07-16T16:51:19Z

Rebased main, which includes #148

roobre · 2021-07-16T16:51:59Z

Tests are failing due to missing entityName entry in MetricSets, wether this is required or not is being investigated with @newrelic/caos.

invidian · 2021-07-19T21:36:19Z

src/client/cached.go

@@ -171,7 +170,7 @@ func (d *MultiDiscoveryCacher) discoverAndCache(timeout time.Duration) ([]HTTPCl
 		err = d.Storage.Write(d.StorageKey, toCache)
 	}
 	if err != nil {
-		d.Logger.WithError(err).Warnf("while storing %q in the cache", d.StorageKey)
+		d.Logger.Warnf("Could not store %q in the cache: %v", d.StorageKey, err)


I wonder if we should log the raw data here, which would help debugging cache issues. Also, it would be nice to have unique error messages. Perhaps Decompose + Write part could be refactored into a function?

src/client/cached.go

src/controlplane/client/discovery.go

src/definition/populate.go

src/definition/populate_test.go

invidian · 2021-07-19T22:05:22Z

I fixed the imports here. Some details could be addressed here, but overall the PR looks good 👍

src/kubernetes.go

src/metric/populate_test.go

src/prometheus/http_test.go

e2e/jsonschema/match.go

src/metric/populate_test.go

src/definition/populate_test.go

invidian

LGTM

roobre · 2021-07-21T10:19:52Z

Holding merge until we perform end to end tests.

gsanchezgavier

LGTM , is wired that the version of the sdk we were using was kind of "almost" v3 from what i could read in the docs for the sdk , but still the protocol version on the json output was 2 and i saw you change it to 3, this is the most scary part to me, and how entities behave.

roobre · 2021-07-21T12:57:02Z

@gsanchezgavier There is a small change on how the entityName is generated. Previously, the SDK did it, and now that generation falls on the agent side. However I checked and they are using the same strategy, so I don't expect breakage there.

Before release, we can upgrade the canaries cluster to this version and ensure that pods and containers keep the same entity IDs and names.

invidian

Just some small nits, overall LGTM. Nice work @roobre!

cmd/kubernetes-discovery/main.go

src/ksm/client/discovery.go

paologallinaharbur · 2021-08-25T07:40:14Z

@roobre is this blocked by any task? there is the risk of forgetting about it

invidian · 2021-10-05T08:47:21Z

I suggest we merge #227 first.

invidian · 2021-10-05T10:09:13Z

Squashed commits and rebased on latest main. It's ready to review 🎉

e2e/jsonschema/match.go

src/definition/populate.go

roobre · 2021-10-05T13:07:04Z

✅ roobre approved these changes on 5 Oct

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> Co-authored-by: Roberto Santalla <roobre@roobre.es>

New SDK version does not need it anymore. Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>

invidian

LGTM. Had to merge logger changes with the first PR, as otherwise code did not compile.

roobre force-pushed the sdk-v3 branch from eccd31a to 2f87b4f Compare July 16, 2021 12:15

roobre force-pushed the sdk-v3 branch from 2f87b4f to b3a7e3e Compare July 16, 2021 16:50

roobre force-pushed the sdk-v3 branch from b3a7e3e to 2bc7cce Compare July 19, 2021 14:54

roobre requested review from invidian and a team July 19, 2021 16:50

invidian reviewed Jul 19, 2021

View reviewed changes

roobre force-pushed the sdk-v3 branch from 919737e to 6e1cb1c Compare July 20, 2021 07:48

invidian reviewed Jul 20, 2021

View reviewed changes

roobre mentioned this pull request Jul 20, 2021

main: refactor main package #163

Closed

roobre force-pushed the sdk-v3 branch 2 times, most recently from e37f717 to cd32a8a Compare July 20, 2021 12:05

invidian reviewed Jul 20, 2021

View reviewed changes

roobre force-pushed the sdk-v3 branch 2 times, most recently from d1c6187 to 7f589cc Compare July 20, 2021 13:07

invidian approved these changes Jul 20, 2021

View reviewed changes

gsanchezgavier approved these changes Jul 21, 2021

View reviewed changes

invidian approved these changes Jul 22, 2021

View reviewed changes

roobre force-pushed the sdk-v3 branch 3 times, most recently from e5dbded to 44be8ad Compare July 28, 2021 08:43

invidian approved these changes Jul 28, 2021

View reviewed changes

cmd/kubernetes-discovery/main.go Outdated Show resolved Hide resolved

src/ksm/client/discovery.go Outdated Show resolved Hide resolved

roobre mentioned this pull request Aug 24, 2021

Bump github.com/newrelic/infra-integrations-sdk from 2.0.1-0.20180410150501-14a5386f9150+incompatible to 2.1.0+incompatible #117

Closed

davidgit assigned roobre Aug 27, 2021

invidian mentioned this pull request Oct 5, 2021

Prepare for SDKv3 migration #227

Merged

invidian mentioned this pull request Oct 5, 2021

src/client: don't use WithError() logger method #228

Merged

invidian force-pushed the sdk-v3 branch from 44be8ad to 55d4602 Compare October 5, 2021 10:08

roobre commented Oct 5, 2021

View reviewed changes

e2e/jsonschema/match.go Outdated Show resolved Hide resolved

roobre commented Oct 5, 2021

View reviewed changes

src/definition/populate.go Outdated Show resolved Hide resolved

invidian force-pushed the sdk-v3 branch 2 times, most recently from 043561a to 44d6c4e Compare October 5, 2021 12:51

invidian and others added 2 commits October 5, 2021 15:21

Migrate to SDK v3

cb4899c

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com> Co-authored-by: Roberto Santalla <roobre@roobre.es>

e2e/jsonschema: don't merge metrics by hand

678285b

New SDK version does not need it anymore. Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>

invidian force-pushed the sdk-v3 branch from 44d6c4e to 678285b Compare October 5, 2021 13:21

invidian approved these changes Oct 5, 2021

View reviewed changes

roobre merged commit 5044159 into main Oct 5, 2021

roobre deleted the sdk-v3 branch October 5, 2021 15:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to SDK v3 #160

Migrate to SDK v3 #160

roobre commented Jul 16, 2021 •

edited

Loading

roobre commented Jul 16, 2021

roobre commented Jul 16, 2021

roobre commented Jul 16, 2021 •

edited

Loading

invidian Jul 19, 2021

invidian commented Jul 19, 2021

invidian left a comment

roobre commented Jul 21, 2021

gsanchezgavier left a comment

roobre commented Jul 21, 2021

invidian left a comment

paologallinaharbur commented Aug 25, 2021

invidian commented Oct 5, 2021

invidian commented Oct 5, 2021

roobre commented Oct 5, 2021

invidian left a comment

Migrate to SDK v3 #160

Migrate to SDK v3 #160

Conversation

roobre commented Jul 16, 2021 • edited Loading

roobre commented Jul 16, 2021

roobre commented Jul 16, 2021

roobre commented Jul 16, 2021 • edited Loading

invidian Jul 19, 2021

Choose a reason for hiding this comment

invidian commented Jul 19, 2021

invidian left a comment

Choose a reason for hiding this comment

roobre commented Jul 21, 2021

gsanchezgavier left a comment

Choose a reason for hiding this comment

roobre commented Jul 21, 2021

invidian left a comment

Choose a reason for hiding this comment

paologallinaharbur commented Aug 25, 2021

invidian commented Oct 5, 2021

invidian commented Oct 5, 2021

roobre commented Oct 5, 2021

invidian left a comment

Choose a reason for hiding this comment

roobre commented Jul 16, 2021 •

edited

Loading

roobre commented Jul 16, 2021 •

edited

Loading