Skip to content

fix(stackdriver): pull mesh uid from config#18655

Merged
istio-testing merged 2 commits intoistio:masterfrom
douglas-reid:better-mesh-id-for-stackdriver
Nov 7, 2019
Merged

fix(stackdriver): pull mesh uid from config#18655
istio-testing merged 2 commits intoistio:masterfrom
douglas-reid:better-mesh-id-for-stackdriver

Conversation

@douglas-reid
Copy link
Copy Markdown
Contributor

@douglas-reid douglas-reid commented Nov 5, 2019 •
edited
Loading

This PR updates the logic for mesh_uid calculation for the stackdriver adapter (in particular, metrics and edges handling).

The prior design was to use the GCE metadata server, but plans change. This now accepts the argument as a config-time param.

Fixes #17952.

Signed-off-by: Douglas Reid douglas-reid@users.noreply.github.com

@douglas-reid
fix(stackdriver): pull mesh uid from config
7e943c8 
Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Nov 5, 2019
@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Nov 5, 2019
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 5, 2019
@douglas-reid
Copy link
Copy Markdown
Contributor Author

douglas-reid commented Nov 5, 2019

@geeknoid what do I do about this failed gencheck_istio ? I ran make gen on my local copy and added those files to this commit. I'm not sure how to next proceed.

@douglas-reid
Copy link
Copy Markdown
Contributor Author

douglas-reid commented Nov 5, 2019

/test gencheck_istio

@douglas-reid
Copy link
Copy Markdown
Contributor Author

douglas-reid commented Nov 6, 2019

/test integ-security-k8s-tests_istio

@douglas-reid douglas-reid marked this pull request as ready for review November 6, 2019 22:49
@douglas-reid douglas-reid requested a review from a team November 6, 2019 22:49
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Nov 6, 2019
@douglas-reid
Copy link
Copy Markdown
Contributor Author

douglas-reid commented Nov 6, 2019

@bianpengyuan @mandarjog can you please take a look? this is needed to properly populate mesh id (and will be needed for a backport to 1.2, 1.3, 1.4).

Big questions: OK to override mesh_uid in metrics (or should it only be if that isn't already set)? Should we do something similar for logs?

@bianpengyuan
Copy link
Copy Markdown
Contributor

bianpengyuan commented Nov 6, 2019

If we are passing in the mesh_uid from config instead of fetching from metadata server during runtime, why not just config metric labels and log labels directly?

@douglas-reid
Copy link
Copy Markdown
Contributor Author

douglas-reid commented Nov 6, 2019

@bianpengyuan the idea, as I understood it, was to simplify the number of places where that config had to be set. and we still need a (backwards-compatible) way to set the mesh_uid for edges too (which the template does not include).

@douglas-reid
rerun make gen
5e87770 
Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
@douglas-reid
Copy link
Copy Markdown
Contributor Author

douglas-reid commented Nov 7, 2019

/test integ-istioio-k8s-tests_istio

@istio-testing istio-testing merged commit d262f6b into istio:master Nov 7, 2019
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Nov 7, 2019

In response to a cherrypick label: #18655 failed to apply on top of branch "release-1.4":

Using index info to reconstruct a base tree...
M	mixer/adapter/stackdriver/contextgraph/contextgraph.go
M	mixer/adapter/stackdriver/contextgraph/contextgraph_test.go
M	mixer/adapter/stackdriver/helper/common.go
M	mixer/adapter/stackdriver/helper/common_test.go
M	mixer/adapter/stackdriver/log/log_test.go
M	mixer/adapter/stackdriver/metric/metric.go
M	mixer/adapter/stackdriver/metric/metric_test.go
M	mixer/adapter/stackdriver/stackdriver.go
M	mixer/adapter/stackdriver/trace/trace_integration_test.go
M	mixer/adapter/stackdriver/trace/trace_test.go
Falling back to patching base and 3-way merge...
Auto-merging mixer/adapter/stackdriver/stackdriver.go
Auto-merging mixer/adapter/stackdriver/metric/metric_test.go
CONFLICT (content): Merge conflict in mixer/adapter/stackdriver/metric/metric_test.go
Auto-merging mixer/adapter/stackdriver/metric/metric.go
CONFLICT (content): Merge conflict in mixer/adapter/stackdriver/metric/metric.go
Auto-merging mixer/adapter/stackdriver/helper/common_test.go
CONFLICT (content): Merge conflict in mixer/adapter/stackdriver/helper/common_test.go
Auto-merging mixer/adapter/stackdriver/contextgraph/contextgraph_test.go
CONFLICT (content): Merge conflict in mixer/adapter/stackdriver/contextgraph/contextgraph_test.go
Auto-merging mixer/adapter/stackdriver/contextgraph/contextgraph.go
CONFLICT (content): Merge conflict in mixer/adapter/stackdriver/contextgraph/contextgraph.go
error: Failed to merge in the changes.
Patch failed at 0001 fix(stackdriver): pull mesh uid from config

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Nov 7, 2019

@douglas-reid: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
integ-istioio-k8s-tests_istio 5e87770 link /test integ-istioio-k8s-tests_istio
Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

sdake pushed a commit to sdake/istio that referenced this pull request Dec 1, 2019
@douglas-reid
fix(stackdriver): pull mesh uid from config (istio#18655)
643fb4e 
* fix(stackdriver): pull mesh uid from config

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>

* rerun make gen

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
@douglas-reid douglas-reid mentioned this pull request Dec 4, 2019
Merged
@douglas-reid douglas-reid mentioned this pull request Dec 4, 2019
Merged
douglas-reid added a commit to douglas-reid/istio that referenced this pull request Dec 5, 2019
@douglas-reid
fix(stackdriver): pull mesh uid from config (istio#18655)
a4d23e4 
* fix(stackdriver): pull mesh uid from config

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>

* rerun make gen

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
douglas-reid added a commit to douglas-reid/istio that referenced this pull request Dec 5, 2019
@douglas-reid
fix(stackdriver): pull mesh uid from config (istio#18655)
c2a6fa1 
* fix(stackdriver): pull mesh uid from config

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>

* rerun make gen

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
istio-testing pushed a commit that referenced this pull request Dec 11, 2019
@douglas-reid @istio-testing
fix(stackdriver): pull mesh uid from config (#18655) (#19401)
a1fa70f 
* fix(stackdriver): pull mesh uid from config

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>

* rerun make gen

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
istio-testing pushed a commit that referenced this pull request Dec 22, 2019
@douglas-reid @istio-testing
fix(stackdriver): pull mesh uid from config (#18655) (#19402)
d5f7e40 
* fix(stackdriver): pull mesh uid from config

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>

* rerun make gen

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
brian-avery pushed a commit to brian-avery/istio that referenced this pull request Mar 4, 2020
@douglas-reid
fix(stackdriver): pull mesh uid from config (istio#18655) (istio#19401)
01d9af6 
* fix(stackdriver): pull mesh uid from config

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>

* rerun make gen

Signed-off-by: Douglas Reid <douglas-reid@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@bianpengyuan bianpengyuan bianpengyuan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/extensions and telemetry cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Stackdriver mixer adapter does not support user-configured mesh_uid in contextgraph (but does in metrics and logs)

5 participants

@douglas-reid @bianpengyuan @istio-testing @googlebot @istio-policy-bot