Skip to content

Batch Metrics Exported From the OTLP Exporter#626

Merged
jmacd merged 11 commits intoopen-telemetry:masterfrom
MrAlias:batch-otlp-metric
Apr 15, 2020
Merged

Batch Metrics Exported From the OTLP Exporter#626
jmacd merged 11 commits intoopen-telemetry:masterfrom
MrAlias:batch-otlp-metric

Conversation

@MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Apr 8, 2020

Batches ResourceMetrics sent to the collector by Resource, InstrumentationLibrary, and the metric labels.

Moves the export pipeline to the internal transform package to clean up.

Adds tests to validate batching and expected form of ResourceMetrics exported.

Fixes #549

@MrAlias MrAlias added exporters area:metrics Part of OpenTelemetry Metrics labels Apr 8, 2020
@MrAlias MrAlias added this to the Beta v0.5 milestone Apr 8, 2020
@MrAlias MrAlias self-assigned this Apr 8, 2020
rghetia
rghetia reviewed Apr 9, 2020
View reviewed changes
Copy link
Contributor

@rghetia rghetia left a comment

Choose a reason for hiding this comment

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

Overall LGTM. One comment on timestamp.

krnowak
krnowak previously requested changes Apr 9, 2020
View reviewed changes
exporters/otlp/internal/transform/metric.go
defer close(out)
// No selected needed since errc is buffered.
errc <- cps.ForEach(func(r export.Record) error {
select {
Copy link
Member

@krnowak krnowak Apr 9, 2020

Choose a reason for hiding this comment

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

Is it possible that timeout, deadline or explicit cancellation happens and the select will still pick the out <- r case, because select provides no ordering guarantees? Is this something to worry about?

Copy link
Contributor Author

@MrAlias MrAlias Apr 9, 2020

Choose a reason for hiding this comment

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

Correct, it is possible for the context Done channel to be closed and yet the out <- r communication operation proceeds. This means that this (and the transform and sync) goroutines will potentially preform unnecessary work before exiting. I think this is not something to worry about. The "pseudo-random selection" process means this case will be reached and these goroutines closed eventually, albeit with some overhead. There is no race condition, which is often the worry I have when this question comes up.

@MrAlias
Copy link
Contributor Author

MrAlias commented Apr 15, 2020

@krnowak: wondering if you have time to take another look. I think you're the last hold out.

@MrAlias MrAlias mentioned this pull request Apr 15, 2020
Closed
@jmacd jmacd dismissed krnowak’s stale review April 15, 2020 19:02

I think this was addressed.

@jmacd jmacd merged commit ebc245b into open-telemetry:master Apr 15, 2020
@MrAlias MrAlias deleted the batch-otlp-metric branch April 15, 2020 19:45
@MrAlias MrAlias mentioned this pull request May 19, 2020
Closed
@aerfio-renovate-bot aerfio-renovate-bot bot mentioned this pull request May 12, 2025
Merged
1 task
MrAlias added a commit that referenced this pull request May 23, 2025
@MrAlias
Semconv v1.34.0 (#6812)
e587b18 
Resolve #6811 
Resolve #6691

No migration documentation is added. This does not contain any breaking
changes between v1.34.0 and v1.33.0 for the Go API.

## [`v1.34.0` semantic convention release
notes](https://github.com/open-telemetry/semantic-conventions/releases/tag/v1.34.0):

<h3>🛑 Breaking changes 🛑</h3>
<ul>
<li>
<p><code>all</code>: Convert deprecated text to structured format. (<a
href="open-telemetry/semantic-conventions#2047"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/2047/hovercard">#2047</a>)<br>
This is a breaking change from the schema perspective, but does not
change anything for instrumentations or the end users. It breaks
compatibility with the (old) <a
href="open-telemetry/build-tools#322"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/build-tools/issues/322/hovercard">code
generation tooling</a>. Please use <a
href="https://github.com/open-telemetry/weaver">weaver</a> to generate
Semantic Conventions markdown or code.</p>
</li>
<li>
<p><code>feature_flag</code>: Move the evaluated value from the event
body to attributes (<a
href="open-telemetry/semantic-conventions#1990"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1990/hovercard">#1990</a>)</p>
</li>
<li>
<p><code>process</code>: Require sensitive data sanitization for
<code>process.command_args</code> and <code>process.command_line</code>
(<a
href="open-telemetry/semantic-conventions#626"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/626/hovercard">#626</a>)</p>
</li>
</ul>
<h3>💡 Enhancements 💡</h3>
<ul>
<li><code>docs</code>: Document system-specific naming conventions (<a
href="open-telemetry/semantic-conventions#608"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/608/hovercard">#608</a>,
<a
href="open-telemetry/semantic-conventions#1494"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/1494/hovercard">#1494</a>,
<a
href="open-telemetry/semantic-conventions#1708"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1708/hovercard">#1708</a>)</li>
<li><code>gen-ai</code>: Add <code>gen_ai.conversation.id</code>
attribute (<a
href="open-telemetry/semantic-conventions#2024"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/2024/hovercard">#2024</a>)</li>
<li><code>all</code>: Renames all <code>resource.*</code> groups to be
<code>entity.*</code> (<a
href="open-telemetry/semantic-conventions#2244"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/2244/hovercard">#2244</a>)<br>
Part of <a class="issue-link js-issue-link" data-error-text="Failed to
load title" data-id="2885075816" data-permission-text="Title is private"
data-url="open-telemetry/opentelemetry-specification#4436"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/opentelemetry-specification/issues/4436/hovercard"
href="https://github.com/open-telemetry/opentelemetry-specification/issues/4436">open-telemetry/opentelemetry-specification#4436</a></li>
<li><code>aws</code>: Added new AWS attributes for various services
including SQS, SNS, Bedrock, Step Functions, Secrets Manager and Kineses
(<a
href="open-telemetry/semantic-conventions#1794"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/1794/hovercard">#1794</a>)</li>
<li><code>cloud</code>: Broaden <code>cloud.region</code> definition to
explicitly cover both resource location and targeted destination. (<a
href="open-telemetry/semantic-conventions#2142"
data-hovercard-type="issue"
data-hovercard-url="/open-telemetry/semantic-conventions/issues/2142/hovercard">#2142</a>)</li>
<li><code>network</code>: Stabilize <code>network.transport</code> enum
value <code>quic</code>. (<a
href="open-telemetry/semantic-conventions#2275"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/2275/hovercard">#2275</a>)</li>
</ul>
<h3>🧰 Bug fixes 🧰</h3>
<ul>
<li><code>db</code>: Fix the <code>db.system.name</code> attribute value
for MySQL which was incorrectly pointing to
<code>microsoft.sql_server</code>. (<a
href="open-telemetry/semantic-conventions#2276"
data-hovercard-type="pull_request"
data-hovercard-url="/open-telemetry/semantic-conventions/pull/2276/hovercard">#2276</a>)</li>
</ul>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmacd jmacd jmacd approved these changes

@lizthegrey lizthegrey Awaiting requested review from lizthegrey

@paivagustavo paivagustavo Awaiting requested review from paivagustavo

@tedsuo tedsuo Awaiting requested review from tedsuo

+2 more reviewers

@rghetia rghetia rghetia approved these changes

@krnowak krnowak krnowak left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@MrAlias MrAlias

Labels

area:metrics Part of OpenTelemetry Metrics

Projects

None yet

Milestone

Implement v0.5.0 Specification

Development

Successfully merging this pull request may close these issues.

Batch Metrics Exported From the OTLP Exporter

4 participants

@MrAlias @krnowak @jmacd @rghetia