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

Sql for .net framework should call Enrich #4161

Closed
wants to merge 2 commits into from

Conversation

davhdavh
Copy link

@davhdavh davhdavh commented Feb 7, 2023

Fixes #4080

Changes

Call Enrich in the approximately the same place for .net framework as in .net.
Update docs to fix naming of .nets
Update docs to mention the difference in 3rd parameter.

@davhdavh davhdavh requested a review from a team February 7, 2023 11:27
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 7, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.


try
{
this.options.Enrich?.Invoke(activity, "OnCustom", eventData.Payload);
Copy link
Member

Choose a reason for hiding this comment

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

Is payload useful to anyone? IIRC we didn't add this because it is just an array of a few strings and there was no way to get at the raw request (command object).

Copy link
Author

Choose a reason for hiding this comment

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

Nothing I am aware of, I cannot think of any reason to use the SqlCommand either, but it encapsulates the same idea as the SqlCommand that is passed from .NET in providing the raw data from with the activity is created from.

Copy link
Member

Choose a reason for hiding this comment

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

if there is no use case, then we dont need Enrich for .NET framework, right? (thats the current behavior)

Copy link
Author

Choose a reason for hiding this comment

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

Just because I cannot see a usecase for the 3rd parameter. That doesn't mean there is no usecase at all. E.g. i want the activity.Displayname to be the same no matter what the underlaying db name is.

Copy link
Member

Choose a reason for hiding this comment

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

to change activity.DisplayName, you can use ActivityProcessor. The whole Enrich feature was provided to provide easy access to raw objects from which users can extract additional information. In case of SQL on .NET FW, the raw object(payload) do not have anything not already parsed out by default.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.

@github-actions github-actions bot added the Stale Issues and pull requests which have been flagged for closing due to inactivity label Feb 16, 2023
@github-actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Issues and pull requests which have been flagged for closing due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sql for .net framework never calls Enrich
3 participants