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

More details about trace data in logging exporter #609

Merged
merged 10 commits into from
Mar 13, 2020

Conversation

pmm-sumo
Copy link
Contributor

@pmm-sumo pmm-sumo commented Mar 10, 2020

Description:

The exporter outputs data in a more verbose format when in debug mode, which allows to easily inspect what spans are being passed using the console.

Additionally, it includes sampling, so by default the console should never get too much data

The logger config is changed to use console encoding

Sample output:

2020-03-13T19:50:09.830Z	INFO	loggingexporter/logging_exporter.go:147	TraceData with 1 spans, node service: frontend, resource "" (1 labels)
Resource labels:
     -> ip: 10.1.1.177
            HostName: hotrod
                 PID: 0
    Library language: LANGUAGE_UNSPECIFIED
Core library version:
    Exporter version: Jaeger-Go-2.15.1dev
Node attributes:
     -> ip: 10.1.1.177
     -> client-uuid: 526d676893250972
	{"exporter": "logging"}
2020-03-13T19:50:09.830Z	DEBUG	loggingexporter/logging_exporter.go:187	Span #0
    Trace ID       : 00000000000000002bb7316b5625e704
    Parent ID      :
    ID             : 2bb7316b5625e704
    Name           : HTTP GET /
    Kind           : SERVER
    Start time     : seconds:1584129009 nanos:56055000
    End time       : seconds:1584129009 nanos:56107000
    Status code    : 0
    Status message :
    Span attributes:
         -> sampler.param: bool_value:true
         -> span.kind: server
         -> http.method: GET
         -> http.url: /
         -> component: net/http
         -> http.status_code: int_value:200
         -> sampler.type: const
	{"exporter": "logging"}

Link to tracking Issue: #610

Testing: Basic unit tests for config and trace data added. Manually tested in an environment with data coming in various formats (Jaeger, Zipkin, OpenCensus)

Documentation: Exporters README updated

@tigrannajaryan
Copy link
Member

tigrannajaryan commented Mar 10, 2020

@pmm-sumo thank you for the contribution.

To ensure smoother experience please read https://github.com/open-telemetry/opentelemetry-collector/blob/master/CONTRIBUTING.md

We ask new contributors to create issues and discuss with maintainers first before submitting PRs. This will ensure better experience for everyone, especially in case the suggested feature is not going to be accepted.

This PR looks useful (from the description) and has a good chance to get accepted. I did not have a detailed look since the build fails. Please submit an issue describing the intent and we will take it from there.

@codecov-io
Copy link

codecov-io commented Mar 10, 2020

Codecov Report

Merging #609 into master will increase coverage by 0.02%.
The diff coverage is 81.70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #609      +/-   ##
==========================================
+ Coverage   75.32%   75.34%   +0.02%     
==========================================
  Files         142      142              
  Lines        9721     9790      +69     
==========================================
+ Hits         7322     7376      +54     
- Misses       2075     2085      +10     
- Partials      324      329       +5     
Impacted Files Coverage Δ
exporter/loggingexporter/logging_exporter.go 81.70% <78.26%> (-18.30%) ⬇️
exporter/loggingexporter/factory.go 67.44% <100.00%> (+4.28%) ⬆️

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 275a932...fbc49ac. Read the comment docs.

@pmm-sumo
Copy link
Contributor Author

@pmm-sumo thank you for the contribution.

To ensure smoother experience please read https://github.com/open-telemetry/opentelemetry-collector/blob/master/CONTRIBUTING.md

We ask new contributors to create issues and discuss with maintainers first before submitting PRs. This will ensure better experience for everyone, especially in case the suggested feature is not going to be accepted.

This PR looks useful (from the description) and has a good chance to get accepted. I did not have a detailed look since the build fails. Please submit an issue describing the intent and we will take it from there.

Thank you @tigrannajaryan and apologies for missing the process. I filled the issue as #610

The PR is something I found useful for my purposes and thought it is worth to be shared with the community. It makes several changes to how the logging exporter was working currently and it focuses solely on tracing as of now. If you find it useful, I will be glad to extend it for metrics too or do any other improvements you find helpful

exporter/README.md Outdated Show resolved Hide resolved
exporter/README.md Outdated Show resolved Hide resolved
exporter/loggingexporter/factory.go Outdated Show resolved Hide resolved
exporter/loggingexporter/factory.go Outdated Show resolved Hide resolved
exporter/loggingexporter/logging_exporter.go Outdated Show resolved Hide resolved
exporter/loggingexporter/logging_exporter.go Outdated Show resolved Hide resolved
exporter/loggingexporter/logging_exporter.go Outdated Show resolved Hide resolved
exporter/loggingexporter/logging_exporter.go Show resolved Hide resolved
testbed/go.sum Outdated Show resolved Hide resolved
The exporter outputs data in a more verbose format, which allows to easily inspect what spans are being passed in the console.

Additionally, it includes sampling, so by default the console should never get too much data
Previously, there was only one effect loglevel of logging exporter. Currently, debug
is much more verbose and should be used for... debugging. For other purposes, into is sufficient.
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

LGTM, except please bring back logging of config.Name().

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

Thanks @pmm-sumo LGTM, just two very nit suggestions and please use the tag "exporter" to log config.Name()

exporter/loggingexporter/config.go Outdated Show resolved Hide resolved
exporter/loggingexporter/config.go Outdated Show resolved Hide resolved
@pmm-sumo
Copy link
Contributor Author

Many thanks @tigrannajaryan and @pjanotti for the review.

I brought back config.Name(). The example of the log output in the PR description was updated.

I was thinking about making similar PR for metrics too, what do you think?

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

Thanks @pmm-sumo!

@pjanotti
Copy link
Contributor

@pmm-sumo

I was thinking about making similar PR for metrics too, what do you think?

That will be nice, just a warning that there are many PRs and we (or perhaps I) may be slow to reply.

Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

Thank you.

@tigrannajaryan tigrannajaryan merged commit 8693f0f into open-telemetry:master Mar 13, 2020
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
* translatesfx: handle SA regex filters

SA allows you to specify a regexp filter by wrapping it in forward slashes.
This changes detects that and translates it into an expr expression.

For example the SA directive:

metricsToExclude:
  - metricName: /vsphere\.cpu_\w*_percent/

should translate into:

processors:
  filter:
    metrics:
      exclude:
        match_type: expr
        expressions:
        - MetricName matches "vsphere\\.cpu_\\w*_percent"

* translatesfx: don't consider a single '/' a regex filter
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024
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.

4 participants