Skip to content

docs: update Jaeger integration#5398

Merged
bmorelli25 merged 2 commits intoelastic:masterfrom
bmorelli25:update-jaeger-docs
Jun 2, 2021
Merged

docs: update Jaeger integration#5398
bmorelli25 merged 2 commits intoelastic:masterfrom
bmorelli25:update-jaeger-docs

Conversation

@bmorelli25
Copy link
Copy Markdown
Member

@bmorelli25 bmorelli25 commented Jun 2, 2021 •
edited
Loading

Motivation/summary

This PR updates the Jaeger integration documentation:

  • Removes HTTP instructions from the Jaeger integration guide
  • Separates ESS and self-managed instructions
  • Deprecates Jaeger Clients via thrift over HTTP instructions and moves them to a collapsed-by-default block
  • Deprecates Jaeger configuration reference

Doc preview

Notes

Tested on ESS with hotrod:

docker run --rm -it --name jaeger-agent --network apm-integration-testing -p6831:6831/udp \
-e REPORTER_GRPC_HOST_PORT=my.cloud.es.io:443 \
-e REPORTER_GRPC_TLS_ENABLED=true \
-e AGENT_TAGS="elastic-apm-auth=Bearer <my-token>" \
jaegertracing/jaeger-agent:latest

docker run --rm -it --network apm-integration-testing \
-e JAEGER_AGENT_HOST=jaeger-agent \
-e JAEGER_AGENT_PORT=6831 \
-p8080-8083:8080-8083 jaegertracing/example-hotrod:latest all

The only thing that wasn't clear to me on ESS was sampling. See my question in the comments below.

Related issues

Closes #4621

@bmorelli25 bmorelli25 self-assigned this Jun 2, 2021
@ghost
Copy link
Copy Markdown

ghost commented Jun 2, 2021 •
edited by ghost
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Started by user Brandon Morelli

  • Start Time: 2021-06-02T04:27:56.074+0000

  • Duration: 41 min 13 sec

  • Commit: f8a546b

Test stats 🧪

Test Results
Failed 0
Passed 6142
Skipped 120
Total 6262

Trends 🧪

Image of Build Times

Image of Tests

bmorelli25
bmorelli25 commented Jun 2, 2021
View reviewed changes
docs/tab-widgets/jaeger-sampling.asciidoc
Comment on lines +1 to +4
// tag::ess[]
Visit the {kibana-ref}/agent-configuration.html[Agent configuration] page in the APM app to add a new sampling rate.

// end::ess[]
Copy link
Copy Markdown
Member Author

@bmorelli25 bmorelli25 Jun 2, 2021

Choose a reason for hiding this comment

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

Does central config sampling in ESS JustWork™️? I changed the sampling rate but didn't see anything in the Jaeger client logs. Is there an easy way to determine if the sampling rate is applied? I also tried looking at transaction.sampled, but couldn't spot an obvious difference.

Copy link
Copy Markdown
Member

@axw axw Jun 2, 2021

Choose a reason for hiding this comment

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

It should. Were you testing with HotROD? It disables remote sampling by default: https://github.com/jaegertracing/jaeger/blob/c0fb781662eacb4ec6f544e78d081528541c2473/examples/hotrod/pkg/tracing/init.go#L38-L39

(The Jaeger client default is remote: https://github.com/jaegertracing/jaeger-client-go/blob/398f7a767754a7dd438d6ff2f8631698bf913696/config/config.go#L238-L243, but this app overrides it)

Copy link
Copy Markdown
Member Author

@bmorelli25 bmorelli25 Jun 2, 2021

Choose a reason for hiding this comment

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

Aha! That's what it was. Thanks for the clarification.

@bmorelli25 bmorelli25 requested a review from axw June 2, 2021 04:27
axw
axw approved these changes Jun 2, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@axw axw 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, thanks!

@bmorelli25 bmorelli25 merged commit 31371a1 into elastic:master Jun 2, 2021
@bmorelli25 bmorelli25 deleted the update-jaeger-docs branch June 2, 2021 20:50
mergify bot pushed a commit that referenced this pull request Jun 2, 2021
@bmorelli25
docs: update Jaeger integration (#5398)
a4fe914 
(cherry picked from commit 31371a1)
@mergify mergify bot mentioned this pull request Jun 2, 2021
Merged
mergify bot pushed a commit that referenced this pull request Jun 2, 2021
@bmorelli25
docs: update Jaeger integration (#5398)
314d091 
(cherry picked from commit 31371a1)
@mergify mergify bot mentioned this pull request Jun 2, 2021
Merged
bmorelli25 added a commit that referenced this pull request Jun 2, 2021
@mergify @bmorelli25
docs: update Jaeger integration (#5398) (#5404)
9ea0241 
Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
bmorelli25 added a commit that referenced this pull request Jun 2, 2021
@mergify @bmorelli25
docs: update Jaeger integration (#5398) (#5405)
6df888c 
Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@axw axw axw approved these changes

Assignees

@bmorelli25 bmorelli25

Labels

docs test-plan-skip v7.13.0 v7.14.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Jaeger] Document support for gRPC on the standard port

3 participants

@bmorelli25 @axw @stuartnelson3