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

Converting Traces and Metrics into logs. #120

Closed
wants to merge 11 commits into from

Conversation

avik-so
Copy link

@avik-so avik-so commented Jun 23, 2020

This OTEP is a proposal for how to standardize the conversion of traces and metrics into Open Telemetry logs.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 23, 2020

CLA Check
The committers are authorized under a signed CLA.

@avik-so
Copy link
Author

avik-so commented Jun 23, 2020

I signed it

* [Issue 398](https://github.com/open-telemetry/opentelemetry-specification/issues/398)
* [Issue 617](https://github.com/open-telemetry/opentelemetry-specification/issues/617)

The [aim here](https://gitter.im/open-telemetry/logs?at=5ee284f2ef5c1c28f0194a89) is to create a standard method to convert a trace or metric into a log for Otel exporters to reduce confusion and increase compatibility.
Copy link
Member

Choose a reason for hiding this comment

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

Is the intent to create a human readable representation of metrics and traces in the logs or a machine readable one? Depending on what the goal is there are likely different choices to be made. It is not clear from this document which one is the goal (or maybe the goal is something else). It would be useful to call this out.

FYI, there is a human-readable version of metric and traces in the logs in the Collector: https://github.com/open-telemetry/opentelemetry-collector/blob/master/exporter/loggingexporter/logging_exporter.go

Copy link
Author

Choose a reason for hiding this comment

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

Good points. After some other feedback I got, I'll make the intention more clear, and also shift the goals a bit. The updates will include multiple export formats depending on intent and purpose, instead of only having a single way of doing things. Also, thank you for making me aware of that prior work.

Copy link
Member

Choose a reason for hiding this comment

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

@avik-so do you intend to update this OTEP?

Copy link
Author

Choose a reason for hiding this comment

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

I do.
Still doing research on my part. I'm happy to get assistance.

Copy link
Contributor

Choose a reason for hiding this comment

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

As for Metrics, see open-telemetry/opentelemetry-proto#181.

This PR amounts to a (rejected) proposal for OTLP. The problem with open-telemetry/opentelemetry-proto#181 is that it was better at describing the input of an aggregation, not the output of an aggregation.

I think if we're logging, the idea is to log raw metric events (i.e., no aggregation). The proposal in that PR has a MetricDescriptor with these fields:

  • Temporality (Cumulative, Delta)
  • Structure (Adding Monotonic, Adding, Grouping)
  • Continuity (Continuous, Snapshot)
    = 12 combinations

These 12 combinations can be thought of as six instruments crossed with cumulative/delta, which completely and accurately describes the current OTel API metric event.

That led to very lengthy explanations of what it means for various kinds aggregation. If we're only considering raw metric events, then this may be a good protocol.

Technical notes

The problem with open-telemetry/opentelemetry-proto#181 may have been predicted by #88 and #93, where more than six potential kinds of instrument were described and then some of them were rejected. See also open-telemetry/opentelemetry-proto#199 believe that a metric descriptor with Structure=Grouping and Temporality=Delta could also be described as the hypothetical "Interval" temporality mentioned there.

There is also a question of whether you can derive system.cpu.utilization or process.cpu.utilization correctly with whatever logging format is proposed: open-telemetry/opentelemetry-specification#819

| trace.start.timestamp | log.Body.StartTimeStamp |
| trace.end.timestamp | log.Body.EndTimeStamp |
| trace.parentId | log.Body.ParentId |
| trace semantic conventions** | log.Resource |
Copy link
Member

Choose a reason for hiding this comment

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

Which semantic conventions? There are conventions for Resources and conventions for Spans. Does this suggest to put Span attributes that are defined in the semantic conventions into the Resource? If so then it is not clear why.

Copy link
Author

Choose a reason for hiding this comment

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

My intent here was the semantic conventions for resources, I missed the fact that they also exist for span attributes.

| metric.AggregationType | log.Attributes.AggregationType |
| metric.Instrumentation | log.Attributes.Instrumentation |

|Open Telemetry Trace field| Open Telemetry Log field|
Copy link
Member

Choose a reason for hiding this comment

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

Probably better to refer to "Span field" instead of "Trace field" since the table lists Span fields.

Copy link
Author

Choose a reason for hiding this comment

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

Since Trace is a high level concept and span isn't, can you point out the differences to me? I obviously missed something here.

| trace.Events | log.Attributes.Events |
| trace.Tracestate | log.Attributes.Tracestate |
| trace.status | log.Attributes.status |
| trace.kind | log.Attributes.kind |
Copy link
Member

Choose a reason for hiding this comment

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

What do we do with the Span Resource? It is missing in this table.

Base automatically changed from master to main January 27, 2021 20:37
@bogdandrutu bogdandrutu requested a review from a team January 27, 2021 20:37
@tigrannajaryan
Copy link
Member

Good points. After some other feedback I got, I'll make the intention more clear, and also shift the goals a bit. The updates will include multiple export formats depending on intent and purpose, instead of only having a single way of doing things. Also, thank you for making me aware of that prior work.

The PR has not been updated and looks like a candidate for closing since no progress has been made for a long while.

@tedsuo tedsuo added the triaged label Feb 6, 2023
@tedsuo
Copy link
Contributor

tedsuo commented Feb 6, 2023

@avik-so I'm going to close this OTEP, please check out the connectors and other work going on in the Collector. If you are still interested in this, please update the OTEP and reopen.

@tedsuo tedsuo closed this Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants