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

Introduce TraceFormatter, add log correlation docs #635

Merged
merged 2 commits into from
Sep 20, 2019
Merged

Conversation

axw
Copy link
Member

@axw axw commented Sep 17, 2019

Introduce TraceFormatter, a function taking a context and returning a fmt.Formatter that can format trace, transaction, and span IDs.

Also, add documentation around log correlation.

Introduce TraceFormatter, a function taking
a context and returning a fmt.Formatter that
can format trace, transaction, and span IDs.
@axw axw requested a review from bmorelli25 September 17, 2019 09:49
@codecov-io
Copy link

codecov-io commented Sep 17, 2019

Codecov Report

Merging #635 into master will increase coverage by 3.22%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #635      +/-   ##
==========================================
+ Coverage   84.55%   87.78%   +3.22%     
==========================================
  Files         122      123       +1     
  Lines        7493     7522      +29     
==========================================
+ Hits         6336     6603     +267     
+ Misses        823      816       -7     
+ Partials      334      103     -231
Impacted Files Coverage Δ
fmt.go 100% <100%> (ø)
breakdown.go 95.89% <0%> (+0.68%) ⬆️
error.go 96.12% <0%> (+0.86%) ⬆️
utils.go 81.91% <0%> (+1.06%) ⬆️
internal/sqlscanner/scanner.go 96.82% <0%> (+1.58%) ⬆️
metrics.go 92.06% <0%> (+1.58%) ⬆️
context.go 94.59% <0%> (+1.8%) ⬆️
module/apmelasticsearch/client.go 95.23% <0%> (+1.9%) ⬆️
internal/apmhttputil/url.go 97.91% <0%> (+2.08%) ⬆️
... and 35 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 41d6c9b...b9eb670. Read the comment docs.

docs/log-correlation.asciidoc Outdated Show resolved Hide resolved
docs/log-correlation.asciidoc Outdated Show resolved Hide resolved
----

You can follow this article in order to ingest JSON-encoded logs with Filebeat:
{blog-ref}how-to-instrument-your-go-app-with-the-elastic-apm-go-agent#logs[How to instrument your Go app with the Elastic APM Go agent].
Copy link
Member

Choose a reason for hiding this comment

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

I agree that the content of this link is better suited for a blog post than documentation, but I'm wondering if you think it's possible/useful to include some of this information in the docs?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would love to see more guided walkthroughs in our docs in general. For the specific case of how to ingest JSON logs using Filebeat, to me it seems more appropriate to add it to FIlebeat docs than here. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's a tough line to toe. Our docs are so difficult to navigate cross-stack that I worry about users getting lost along the way. With this in mind, I've added a small amount of Filebeat information to the central documentation for this feature -- with the hopes that it will be helpful enough for users, and not be too difficult to maintain.

elastic/apm-server#2672

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool, I'll link to that instead then.

Copy link
Member Author

Choose a reason for hiding this comment

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

... of course that's not actually merged yet, so I can't. I'll update the Go docs once that's merged and released.

docs/log-correlation.asciidoc Outdated Show resolved Hide resolved
Copy link
Member

@bmorelli25 bmorelli25 left a comment

Choose a reason for hiding this comment

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

This LGTM. Once all Agents have their documentation created I may come back trough and open a PR with suggested changes to the format/layout to better align with others.

@axw axw merged commit f292236 into elastic:master Sep 20, 2019
@axw axw deleted the log-fmt branch September 20, 2019 09:33
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.

3 participants