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

Adding "slowness" back to Observability tutorial #620

Merged
merged 9 commits into from
Mar 23, 2022

Conversation

paulyuk
Copy link
Contributor

@paulyuk paulyuk commented Mar 22, 2022

Description

Updated Kubernetes deploy manifests to pull the "slow" container image for multiply app, and then updated Readme to filter zipkin by slow requests.

Issue reference

#619

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • The quickstart code compiles correctly
  • You've tested new builds of the quickstart if you changed quickstart code
  • You've updated the quickstart's README if necessary
  • If you have changed the steps for a quickstart be sure that you have updated the automated validation accordingly. All of our quickstarts have annotations that allow them to be executed automatically as code. For more information see mechanical-markdown. For user guide with examples see Examples.

Signed-off-by: Paul Yuknewicz <paulyuk@microsoft.com>
Signed-off-by: Paul Yuknewicz <paulyuk@microsoft.com>
Signed-off-by: Paul Yuknewicz <paulyuk@microsoft.com>
…de will never be slow enough to return traces results!

Signed-off-by: Paul Yuknewicz <paulyuk@microsoft.com>
…ner from GHCR

Signed-off-by: Paul Yuknewicz <paulyuk@microsoft.com>
Signed-off-by: Paul Yuknewicz <paulyuk@microsoft.com>
Signed-off-by: Paul Yuknewicz <paulyuk@microsoft.com>
amulyavarote
amulyavarote previously approved these changes Mar 22, 2022
Signed-off-by: Paul Yuknewicz <paulyuk@microsoft.com>
@paulyuk
Copy link
Contributor Author

paulyuk commented Mar 22, 2022

I would still like an RCA why multiplyapp deployment shows as unchanged in kubernetes/kind, but I have at least achieved the two goals of 1) the tests pass and 2) the observability tutorial delivers on its promise of finding slow requests.

For these reasons I suggest we take and merge this.

@msfussell
Copy link
Member

The 19411 change is simply to avoid the clash between self hosted and K8s if running on the same machine correct?

@paulyuk
Copy link
Contributor Author

paulyuk commented Mar 23, 2022

The 19411 change is simply to avoid the clash between self hosted and K8s if running on the same machine correct?

Yes exactly. This tutorial starts users in the self hosted mode with zipkin, and then transitions into the kubernetes mode, walking them straight into the clash. Given we're port mapping/forwarding anyway I feel this is the ideal solution.

Copy link
Member

@msfussell msfussell left a comment

Choose a reason for hiding this comment

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

LGTM

@msfussell msfussell merged commit 107cd31 into dapr:master Mar 23, 2022
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.

None yet

3 participants