Skip to content

Improve core code performance by replacing sync.Mutex with sync.RWMutex#1494

Closed
wgliang wants to merge 1 commit intoopen-telemetry:mainfrom
wgliang:performance/replacing-mutex-with-rwmutex
Closed

Improve core code performance by replacing sync.Mutex with sync.RWMutex#1494
wgliang wants to merge 1 commit intoopen-telemetry:mainfrom
wgliang:performance/replacing-mutex-with-rwmutex

Conversation

@wgliang
Copy link
Copy Markdown
Member

@wgliang wgliang commented Jan 27, 2021

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 27, 2021 •
edited
Loading

Codecov Report

Merging #1494 (ea6d16f) into master (c066f15) will not change coverage.
The diff coverage is 100.0%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1494   +/-   ##
======================================
  Coverage    79.0%   79.0%           
======================================
  Files         128     128           
  Lines        6685    6685           
======================================
  Hits         5282    5282           
  Misses       1148    1148           
  Partials      255     255
Impacted Files Coverage Δ
internal/global/meter.go 90.3% <100.0%> (ø)
sdk/trace/span.go 91.4% <100.0%> (ø)

Base automatically changed from master to main January 29, 2021 19:39
@MrAlias
Copy link
Copy Markdown
Contributor

MrAlias commented Feb 11, 2021

Can you include benchmarks to show the improvement and motivate the change?

Aneurysm9
Aneurysm9 reviewed Feb 12, 2021
View reviewed changes
Comment thread internal/global/meter.go
func (p *meterProvider) Meter(instrumentationName string, opts ...metric.MeterOption) metric.Meter {
p.lock.Lock()
defer p.lock.Unlock()
p.lock.RLock()
Copy link
Copy Markdown
Member

@Aneurysm9 Aneurysm9 Feb 12, 2021

Choose a reason for hiding this comment

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

This method can modify p.meters, so should this not be a read lock?

@MrAlias
Copy link
Copy Markdown
Contributor

MrAlias commented Mar 4, 2021

This seems abandoned. Please reopen if that is incorrect.

@MrAlias MrAlias closed this Mar 4, 2021
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

@dashpole dashpole Awaiting requested review from dashpole dashpole is a code owner

@evantorrie evantorrie Awaiting requested review from evantorrie

@jmacd jmacd Awaiting requested review from jmacd

@lizthegrey lizthegrey Awaiting requested review from lizthegrey

@MrAlias MrAlias Awaiting requested review from MrAlias MrAlias is a code owner

@XSAM XSAM Awaiting requested review from XSAM XSAM is a code owner

1 more reviewer

@Aneurysm9 Aneurysm9 Aneurysm9 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@wgliang @MrAlias @Aneurysm9