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.

Sign up for GitHub

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

Jump to bottom

Enable OpenTelemetryBridge by default #2140

Merged
merged 19 commits into from
Aug 30, 2023
Merged

Enable OpenTelemetryBridge by default #2140

merged 19 commits into from
Aug 30, 2023

Conversation

Mpdreamz
Copy link
Member

This will enable the Open Telemetry bridge by default. Requiring no additional configuration to pick up
ActivitySource backed instrumentations.

This also moved the OpenTelemetryBridge out of preview

@Mpdreamz
Enable OpenTelemetryBridge by default
a663911 
This will enable the Open Telemetry bridge by default. Requiring no additional configuration to pick up
`ActivitySource` backed instrumentations.

This also moved the OpenTelemetryBridge out of preview
@Mpdreamz Mpdreamz mentioned this pull request Jul 25, 2023
Closed
@sergey-tihon
Copy link

@Mpdreamz can you please check that Sampler for the telemetry bridge transactions considers TransactionSampleRate?

@Mpdreamz
Copy link
Member Author

Mpdreamz commented Aug 4, 2023

@sergey-tihon they should do the ElasticActivityListener uses StartInternalTransaction and StartInternalSpan

apm-agent-dotnet/src/Elastic.Apm/OpenTelemetry/ElasticActivityListener.cs

Line 76 in 02ea126

transaction = _tracer.StartTransactionInternal(activity.DisplayName, "unknown",

Both take the current TransactionSempleRate into account.

Are you observing differently?

@sergey-tihon
Copy link

sergey-tihon commented Aug 4, 2023 •
edited
Loading

I think yes... I had 100% sample rate in Kibana with following config

     "TransactionSampleRate": "0.2",
    "OpenTelemetryBridgeEnabled": true

most transaction came from azsdk-net-Storage.Blobs telemetry
image

10 mins ago I deployed the version with "OpenTelemetryBridgeEnabled": false and so far I see 20% sample rate

@Mpdreamz
Copy link
Member Author

@sergey-tihon took a while to work my way back to this PR, i think I got a grasp on us not respecting the agents sampling decisions and opened up a follow up PR:

#2169

Many many thanks for highlighting this ahead of us shipping this ❤️

stevejgordon
stevejgordon approved these changes Aug 30, 2023
View reviewed changes
Copy link
Contributor

@stevejgordon stevejgordon left a comment

Choose a reason for hiding this comment

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

LGTM with one minor typo suggestion

@Mpdreamz Mpdreamz merged commit 4415339 into main Aug 30, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevejgordon stevejgordon stevejgordon approved these changes

@gregkalapos gregkalapos Awaiting requested review from gregkalapos

Assignees
No one assigned
Labels
agent-dotnet
Projects
APM-Agents (OLD)
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Mpdreamz @sergey-tihon @stevejgordon