Edge mixer template. Stackdriver Adapter: Add support to write to Context Graph #6709

jeffmendoza · 2018-06-28T18:39:54Z

This includes #6408, so I'll close that one.

Add Mixer template for edge instances. These can be used by adapters to construct graphs of the Istio mesh.

Add support to Stackdriver adapter to write to the Stackdriver Context Graph API.

The Context Graph client is not available upstream yet. So vendor it inside the Stackdriver adapter until it is available.

This client is not available upstream yet.

Build edges and entities from workload instance assertions Update edge template to have UIDs Replace traffic cache with edge cache Send from the new caches Send all entities and edges in a batch with the same timestamp Make it build Use generic path to service account Fix epochs Move context API dependencies into a local vendor directory Propagate endpoint setting from config

Fix identification of kubernetes owners and workload instances Moved contextgraph adapter to be a part of stackdriver. Move vendored contextgraph client Fix Gopkg.toml Update testconfig Temp comment out stackdriver trace code, causing panic. Update local test yaml Fix two errors on determining k8s entities from owner/WI names

Don't escape project names Add tests for workloadInstance Only print details if they exist Remove retry, so invalid entities and edges are not continually reasserted Use a 30 second timeout on AssertBatch calls, and plumb through context in function parameters

douglas-reid

mostly nits

douglas-reid · 2018-06-28T18:53:57Z

mixer/adapter/stackdriver/contextgraph/cache.go

+
+// entityCache tracks the entities we've already seen.
+// It is not thread-safe.
+type entityCache struct {


i assume that pkg/cache was insufficient for this purpose?

@quentinmit Can you take a look? Thanks

douglas-reid · 2018-06-28T18:57:12Z

mixer/adapter/stackdriver/contextgraph/cache.go

+
+// edgeCache tracks the edges we've already seen.
+// It is not thread-safe.
+type edgeCache struct {


these (edgeCache and entityCache) look identical. Is there no way to refactor the code so as we don't have identical Flush and AssertAndCheck methods?

Not until generics land in Go 2 ;)

douglas-reid · 2018-06-28T19:01:08Z

mixer/adapter/stackdriver/contextgraph/contextgraph_test.go

+	}
+}
+
+type mockLogger struct{}


can we use mixer/pkg/adapter/test/env.go instead?

or, at the very least, embed adapter.logger in the struct and only implement the needed methods?

Yes, let me try this out.

douglas-reid · 2018-06-28T19:03:52Z

mixer/adapter/stackdriver/contextgraph/send.go

+				}
+			}
+		case t := <-h.sendTick.C:
+			if t.After(lastFlush.Add(9 * time.Minute)) {


should this (9 * time.Minute) be configurable?

No, it is a detail of the ContextGraph API, and wouldn't be good for the user to configure.

Context API requires a refresh every 10m. This is something clients are just supposed to hardcode.

douglas-reid · 2018-06-28T20:30:56Z

mixer/adapter/stackdriver/contextgraph/contextgraph.go

+		traffics:       make(chan trafficAssertion),
+		entitiesToSend: make([]entity, 0),
+		edgesToSend:    make([]edge, 0),
+		sendTick:       time.NewTicker(30 * time.Second),


should 30s be a const, and/or configurable?

I don't think it should be user configurable, but maybe. The ContextGraph API has a very low quota, and expects us to batch up asserts.

douglas-reid · 2018-06-28T20:39:08Z

mixer/adapter/stackdriver/contextgraph/contextgraph.go

+	gax "github.com/googleapis/gax-go"
+
+	"google.golang.org/api/option"
+	contextgraphpb "google.golang.org/genproto/googleapis/cloud/contextgraph/v1alpha1"


style nit: we should separate out the istio imports section from the third-party. these two lines belong with the others.

codecov · 2018-06-28T21:39:56Z

Codecov Report

Merging #6709 into master will decrease coverage by 1%.
The diff coverage is 82%.

@@           Coverage Diff           @@
##           master   #6709    +/-   ##
=======================================
- Coverage      71%     70%   -<1%     
=======================================
  Files         354     355     +1     
  Lines       30711   30610   -101     
=======================================
- Hits        21579   21406   -173     
- Misses       8284    8344    +60     
- Partials      848     860    +12

Impacted Files	Coverage Δ
mixer/adapter/stackdriver/log/log.go	`69% <0%> (ø)`	⬆️
mixer/adapter/stackdriver/metric/metric.go	`87% <0%> (ø)`	⬆️
mixer/adapter/stackdriver/contextgraph/cache.go	`100% <100%> (ø)`
mixer/adapter/stackdriver/helper/common.go	`84% <100%> (ø)`	⬆️
mixer/adapter/stackdriver/trace/trace.go	`89% <100%> (ø)`	⬇️
mixer/adapter/stackdriver/stackdriver.go	`50% <55%> (+1%)`	⬆️
mixer/adapter/stackdriver/contextgraph/workload.go	`76% <76%> (ø)`
...r/adapter/stackdriver/contextgraph/contextgraph.go	`83% <83%> (ø)`
mixer/adapter/stackdriver/contextgraph/send.go	`84% <84%> (ø)`
...olarwinds/internal/papertrail/papertrail_logger.go	`62% <0%> (-18%)`	⬇️
... and 53 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce70b47...f990f0f. Read the comment docs.

douglas-reid · 2018-06-29T22:29:27Z

We need this PR in ASAP to make the 1.0 release cut. We can make small adjustments to the code moving forward, but I see no major blockers in my review.

/lgtm
/approve
/retest

jeffmendoza · 2018-06-30T00:44:46Z

@douglas-reid Yay, all tests pass.

douglas-reid · 2018-06-30T16:01:04Z

/lgtm

istio-testing · 2018-06-30T16:01:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: douglas-reid, jeffmendoza

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [douglas-reid]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jeffmendoza · 2018-06-30T21:07:13Z

/test istio-presubmit

jeffmendoza · 2018-06-30T21:07:58Z

Docker Hub!!

 error pulling image configuration: Get https://production.cloudflare.docker.com/registry-v2/docker/registry/v2/blobs/sha256/21/219e59ae4605f1a61d4f33dbc6dc46f170a8be109b3ba59cba317b7633a87b9b/data?verify=1530379542-2YTxCF70uquWqfVJSgIQqGuQg98%3D: dial tcp: lookup production.cloudflare.docker.com on 10.47.240.10:53: server misbehaving

jeffmendoza and others added 10 commits June 27, 2018 14:19

Add vendored Contextgraph client under Stackdriver.

65b17ce

This client is not available upstream yet.

Initial contextgraph adapter.

3f46115

Put cache code in separate file, track lastSent and lastSeen separately.

abf63a3

Contextgraph cleanup and unit tests.

049b9a7

Add test for entityCache and edgeCache, fix edge case logic.

80bbb8c

Complete adapter validate function.

fbee454

Use context.protocol if it is grpc as well.

a734a09

istio-testing requested review from geeknoid, linsun and sdake June 28, 2018 18:39

jeffmendoza requested review from douglas-reid and removed request for sdake, linsun and geeknoid June 28, 2018 18:39

googlebot added the cla: yes label Jun 28, 2018

jeffmendoza requested a review from bianpengyuan June 28, 2018 18:40

douglas-reid reviewed Jun 28, 2018

View reviewed changes

Use mixer adapter env testing package. Re-order imports

405615a

Update Stackdriver adapter to use cluster location, not VM.

4f416e6

istio-testing assigned douglas-reid Jun 29, 2018

istio-testing added lgtm and removed lgtm labels Jun 29, 2018

jeffmendoza force-pushed the contextgraph branch from a810d36 to 06b2ee0 Compare June 29, 2018 23:38

Lint.

a70ac9f

jeffmendoza force-pushed the contextgraph branch from 06b2ee0 to a70ac9f Compare June 29, 2018 23:45

Remove cluster/zone checks from adapter validator.

f990f0f

istio-testing added the lgtm label Jun 30, 2018

istio-testing merged commit 0ccdef4 into istio:master Jun 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Edge mixer template. Stackdriver Adapter: Add support to write to Context Graph #6709

Edge mixer template. Stackdriver Adapter: Add support to write to Context Graph #6709

jeffmendoza commented Jun 28, 2018

douglas-reid left a comment

douglas-reid Jun 28, 2018

jeffmendoza Jun 28, 2018

douglas-reid Jun 28, 2018

quentinmit Jun 28, 2018

douglas-reid Jun 28, 2018

jeffmendoza Jun 28, 2018

jeffmendoza Jun 28, 2018

douglas-reid Jun 28, 2018

jeffmendoza Jun 28, 2018

quentinmit Jun 28, 2018

douglas-reid Jun 28, 2018

jeffmendoza Jun 28, 2018

douglas-reid Jun 28, 2018

jeffmendoza Jun 28, 2018

jeffmendoza Jun 28, 2018

codecov bot commented Jun 28, 2018 •

edited

Loading

douglas-reid commented Jun 29, 2018

jeffmendoza commented Jun 30, 2018

douglas-reid commented Jun 30, 2018

istio-testing commented Jun 30, 2018

jeffmendoza commented Jun 30, 2018

jeffmendoza commented Jun 30, 2018

Edge mixer template. Stackdriver Adapter: Add support to write to Context Graph #6709

Edge mixer template. Stackdriver Adapter: Add support to write to Context Graph #6709

Conversation

jeffmendoza commented Jun 28, 2018

douglas-reid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jun 28, 2018 • edited Loading

Codecov Report

douglas-reid commented Jun 29, 2018

jeffmendoza commented Jun 30, 2018

douglas-reid commented Jun 30, 2018

istio-testing commented Jun 30, 2018

jeffmendoza commented Jun 30, 2018

jeffmendoza commented Jun 30, 2018

codecov bot commented Jun 28, 2018 •

edited

Loading