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

Example and docs for .NET span profiles #3224

Merged
merged 8 commits into from
Apr 19, 2024

Conversation

aleks-p
Copy link
Contributor

@aleks-p aleks-p commented Apr 16, 2024

  • enables OpenTelemetry based tracing in the .NET ride share example
  • adds the .NET ride share app to examples/tracing/tempo, to demonstrate span profiles in .NET
  • adds docs for .NET span profiles

@aleks-p aleks-p requested review from a team as code owners April 16, 2024 11:28
Comment on lines 54 to 57
.AddProcessor(new PyroscopeSpanProcessor.Builder()
.WithRootSpanOnly(true)
.Build());
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about removing the WithRootSpanOnly option? It should be true by default, and, honestly, I'm not very sure if we do want to make it configurable. We left this in other integrations to avoid breaking changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is true by default, I added it here so that it is more visible. I don't have a strong opinion about it, I will remove the configuration option for now and add it back in the future if needed.

@aleks-p
Copy link
Contributor Author

aleks-p commented Apr 17, 2024

@knylander-grafana could you take a look at the added doc here?

Also, we probably need to update https://grafana.com/docs/grafana/latest/datasources/tempo/configure-tempo-data-source/#trace-to-profiles but it might make sense to remove the list of languages there and rely on the already linked page for this.

Copy link
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

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

Great additions to the docs! Well done. Thank you for updating things.

Copy link
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you for updating the docs.

@knylander-grafana knylander-grafana added type/docs Improvements for doc docs. Used by Docs team for project management backport release/v1.5 This label will backport a merged PR to the release/v1.5 branch labels Apr 19, 2024
Copy link
Contributor

This PR must be merged before a backport PR will be created.

@aleks-p aleks-p merged commit 635b42c into main Apr 19, 2024
18 checks passed
@aleks-p aleks-p deleted the example/dotnet-span-profiles-otel branch April 19, 2024 19:02
github-actions bot pushed a commit that referenced this pull request Apr 19, 2024
* Enable span profiles for the .NET ride share example app

* Add docs for .NET span profiles

* Add missing file

* Bump Pyroscope.OpenTelemetry to 0.2.0

* Update docs to reflect library update

* Apply suggestions from code review

* Apply suggestions from code review

* Unify span profiling doc introduction across languages

---------

Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>
(cherry picked from commit 635b42c)

This comment was marked as outdated.

aleks-p added a commit that referenced this pull request Apr 19, 2024
* Enable span profiles for the .NET ride share example app

* Add docs for .NET span profiles

* Add missing file

* Bump Pyroscope.OpenTelemetry to 0.2.0

* Update docs to reflect library update

* Apply suggestions from code review

* Apply suggestions from code review

* Unify span profiling doc introduction across languages

---------

Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>
(cherry picked from commit 635b42c)

Co-authored-by: Aleksandar Petrov <8142643+aleks-p@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release/v1.5 This label will backport a merged PR to the release/v1.5 branch backport-failed type/docs Improvements for doc docs. Used by Docs team for project management
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants