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

feat: enable traces #3239

Closed
wants to merge 1 commit into from
Closed

Conversation

aldy505
Copy link
Collaborator

@aldy505 aldy505 commented Aug 1, 2024

NOTE: I don't know if the feature flag will be removed in the future. But considering we have errors-only version of Sentry, I don't think this will be removed, at least for the near future. So I'm adding this here.

Some people are already asking about it.

On Github: #3214
On Discord: https://discord.com/channels/621778831602221064/796028405833007104/1260595924774813757, https://discord.com/channels/621778831602221064/796028405833007104/1266233909197410335

This is the "Traces" page:

image

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Copy link

codecov bot commented Aug 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.02%. Comparing base (cd7c460) to head (e9d1648).
Report is 15 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3239      +/-   ##
==========================================
- Coverage   99.01%   98.02%   -0.99%     
==========================================
  Files           3        3              
  Lines         203      203              
==========================================
- Hits          201      199       -2     
- Misses          2        4       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@TheDevMinerTV TheDevMinerTV left a comment

Choose a reason for hiding this comment

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

works on my instance

@gggritso
Copy link
Member

gggritso commented Aug 1, 2024

@k-fish @Zylphrex thoughts?

@gggritso
Copy link
Member

gggritso commented Aug 1, 2024

As an aside, IMO "Beta" features should not be enabled by default. Folks who are interested are free to enable the feature on their instance! Maybe what we need is product documentation to help self-hosted users enable these features?

@Zylphrex
Copy link
Member

Zylphrex commented Aug 1, 2024

As an aside, IMO "Beta" features should not be enabled by default. Folks who are interested are free to enable the feature on their instance! Maybe what we need is product documentation to help self-hosted users enable these features?

Agreed. In general, we should not enable features labeled beta in self hosted instances by default. Users can enable them on a case by case basis if they wish. But we should only enable a feature by default once it has been moved out of beta. In this case, there are some significant redesigns coming up and we would like to address them before we enable the feature by default.

@aldy505
Copy link
Collaborator Author

aldy505 commented Aug 1, 2024

As an aside, IMO "Beta" features should not be enabled by default. Folks who are interested are free to enable the feature on their instance! Maybe what we need is product documentation to help self-hosted users enable these features?

There's always this: https://develop.sentry.dev/self-hosted/#enabling-preview-features

The docs should be on this repo's issue. But since this feature is simple enough to set up, I went in and add this instead.

Compared to other features which requires tweaks here and there, requiring a new consumer container and stuff.

@aldy505 aldy505 closed this Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants