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

Add InProc dependency type and update db target mapping. #23541

Merged
merged 4 commits into from
Aug 26, 2021
Merged

Add InProc dependency type and update db target mapping. #23541

merged 4 commits into from
Aug 26, 2021

Conversation

vishweshbankwar
Copy link
Contributor

  1. InProc type should be used for all the in-process spans (Part B Span.kind == SpanKind.INTERNAL) with parent
  2. For db target:
  • If target derived from peerService or net* attributes is not null, then append Span.dbName (if present) using / delimiter.
  • If target derived from peerService or net* attributes is null, then assign Span.dbName (if present) to target.
  • Default: If none of the above are available, assign Span.dbSystem to target.

@vishweshbankwar vishweshbankwar added the Monitor - Exporter Monitor OpenTelemetry Exporter label Aug 25, 2021
@vishweshbankwar vishweshbankwar marked this pull request as ready for review August 26, 2021 01:14
rajkumar-rangaraj
rajkumar-rangaraj reviewed Aug 26, 2021
View reviewed changes
sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/HttpHelper.cs Outdated
{
string target = tagObjects.GetDependencyTarget(PartBType.Db);
string dbName = AzMonList.GetTagValue(ref tagObjects, SemanticConventions.AttributeDbName)?.ToString();
if (!string.IsNullOrEmpty(target) && !string.IsNullOrEmpty(dbName))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Instead of checking the string content several times. We could enable check once earlier and based on its flag we could validate in if-else.

Copy link
Member

Choose a reason for hiding this comment

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

Good point! I think we probably need some perf benchmark and numbers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - it is in my todo list.

@vishweshbankwar vishweshbankwar merged commit ba52863 into main Aug 26, 2021
@vishweshbankwar vishweshbankwar deleted the vibankwa/Azure.Monitor.OTEL.Exporter/Add-InProc-DependencyType-And-UpdateDBTarget branch August 26, 2021 17:16
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-net that referenced this pull request Apr 19, 2023
CodeGen from PR 23541 in Azure/azure-rest-api-specs
80a6f0d 
Adding a new stable API-version 2023-05-01 (Azure#23541)

* Base commit from 2023-01-01

* Changing version from 2023-01-01 to 2023-05-01

* Adding Hooks changes in the new API version

* Updating readme files of Microsoft.Dataprotection

---------

Co-authored-by: SHASHANK SINGH <shashasingh@microsoft.com>
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-net that referenced this pull request Apr 20, 2023
CodeGen from PR 23647 in Azure/azure-rest-api-specs
fde9743 
Revert "Adding a new stable API-version 2023-05-01 (Azure#23541)" (Azure#23647)

This reverts commit c655f508b77a442f106250c1ced44ee40e110b00.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rajkumar-rangaraj rajkumar-rangaraj rajkumar-rangaraj approved these changes

@reyang reyang reyang approved these changes

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

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

Assignees
No one assigned
Labels
Monitor - Exporter Monitor OpenTelemetry Exporter
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vishweshbankwar @rajkumar-rangaraj @reyang