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

docs: update emojivoto example #129

Merged
merged 6 commits into from
May 2, 2023

Conversation

JamieDanielson
Copy link
Member

This updates the emojivoto example in the getting-started docs:

  • Remove launcher, which is no longer needed after Allocate memory without launcher #40
  • Update instrumentation to use otel-go-instrumentation image instead of keyval/otel-go-agent
  • Simplify paths to kubernetes manifests (use relative instead of absolute)

Note: This instruments gRPC but has the issue referenced in #78 . Also, the docker image has not yet been published so this currently requires a local build. Since those are already known issues, I think this is worth merging in anyway - I'm not sure how helpful it is to show an example of the instrumentation if it's not actually instrumentation from this library.

{"level":"error","ts":1682967887.022351,"caller":"instrumentors/runner.go:88","msg":"error while loading instrumentors, cleaning up","name":"google.golang.org/grpc","error":"field UprobeHttp2ClientCreateHeaderFields: program uprobe_Http2Client_CreateHeaderFields: load program: permission denied: 912: (73) *(u8 *)(r10 -196) = r7: 913: (79) r2 = *(u64 *)( (truncated, 1185 line(s) omitted)","stacktrace":"go.opentelemetry.io/auto/pkg/instrumentors.(*Manager).load\n\t/app/pkg/instrumentors/runner.go:88\ngo.opentelemetry.io/auto/pkg/instrumentors.(*Manager).Run\n\t/app/pkg/instrumentors/runner.go:36\nmain.main\n\t/app/cli/main.go:86\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:250"}

@JamieDanielson JamieDanielson requested a review from a team May 1, 2023 19:28
Copy link
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Looks good for me. Made a suggestion to require the docker image be created locally as part of the example until an official image is available.

docs/getting-started/emojivoto-instrumented.yaml Outdated Show resolved Hide resolved
Copy link
Member

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Thanks @JamieDanielson - just needs a changelog entry 👍🏻

@JamieDanielson
Copy link
Member Author

Thanks @JamieDanielson - just needs a changelog entry 👍🏻

Happy to add, but wasn't sure it needed one since it's just an example update.

@MikeGoldsmith
Copy link
Member

Ah, true - we can skip changelog!

@MrAlias MrAlias mentioned this pull request May 2, 2023
@MrAlias MrAlias merged commit 29ac2af into open-telemetry:main May 2, 2023
@JamieDanielson JamieDanielson deleted the jamie.update-emoji-example branch May 2, 2023 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants