Tracing: Remove UI at Settings > Monitor > Tracing
What does this MR do and why?
This MR aims to solve the issue #351388 (closed)
Remove deprecated tracing UI from Settings > Monitor > Tracing tab
Screenshots or screen recordings
Before | After |
---|---|
![]() |
![]() |
Merge request reports
Activity
Thank you for your contribution to GitLab. We believe that everyone can contribute and contributions like yours are what make GitLab great!
- Our Merge Request Coaches will ensure your contribution is reviewed in a timely manner*.
- If you haven't, please set up a
DANGER_GITLAB_API_TOKEN
. - You can comment
@gitlab-bot label ~"group::<group name>"
to add a group label or@gitlab-bot label ~"type::<type name>"
to add a type label. - If you need help moving the MR forward or finding a reviewer, feel free to ask
@gitlab-bot help
or ping a Merge Request Coach. - Read more on how to get help.
- You can provide feedback on the GitLab Contributor Experience in this survey
This message was generated automatically. You're welcome to improve it.
added Community contribution label
assigned to @anshulriyal
This merge request will be considered part of the quarterly GitLab Hackathon for a chance to win a prize.
Can you make sure this merge request mentions or links to the relevant issue that it's attempting to close?
Thank you for your contribution!
added Hackathon label
Greeting @ameliabauerly , i have made the necessary changes. Although, I haven't deleted this file as i'm not completely sure to destroy the file. Please let me know if further changes are required...
Hey, @anshulriyal! Thank you for contributing!
I believe we are going to do all of the tracing removals behind a feature flag, so we would probably need to merge this behind that feature flag.
@syasonik - I believe I saw that you are working on the feature flag for tracing. Would you be able to help Anshul with the steps necessary for including this MR behind the feature flag you are creating?
Thanks @ameliabauerly! And thank you so much @anshulriyal for opening this MR! This is a great start.
Amelia is correct about the feature flag. grouprespond is essentially disabling this code with a feature flag rather than deleting it in %15.0. So we won't want to merge the deletions until the %15.2 release begins in late June.
I think the main options for this MR are:
- Finish polishing the MR now & resolve any potential conflicts before merging in %15.2
- Pause development for right now, and wait to finish polishing until %15.2
- Close this MR and pick another issue to work on which could offer more immediate payoff
Let me know which approach makes the most sense for you!
Since this is a hackathon MR, I also put together a list of some of the grouprespond issues which might be suitable options.
- The close/reopen buttons at the bottom of an in... (#343237 - closed)
- Hide SLA /Published columns on incident list if... (#353167 - closed)
- User can create on-call rotation with negative ... (#332474 - closed)
- Update issue type tooltip text in the new issue... (#345163)
- User is shifted to the issue navigation when cr... (#334720 - closed)
- Adding a todo for an incident via the incident ... (#352615 - closed)
- Dropdowns on alerts are larger than expected (#347370 - closed)
- On call schedules - existing participants appea... (#328192 - closed)
- Disallow Guest authors to close Incidents (#345394 - closed)
- Persist issue form attributes on Issue Type change (#357045 - closed)
Greetings @syasonik , Thank you for the advice. I believe to pause this here & wait. As we might get more changes in the plan for upcoming weeks or we can close this MR as well. Please let me know what would be the best practice here
We can definitely pause this @anshulriyal!
I'll check in with you in June about picking this up again. We'll want to update the tests and delete the file you linked at that time. I'll put a reminder on my calendar.
Thanks again for your work on this!
@syasonik FYI I think this duplicates !87175 (closed) so you might want to choose one of the two MRs in the end.
Greetings @rymai #351387 (closed) & #351388 (closed) are related, therefore i picked these 2 & raise MR for both. Happy to resolve this MR conflict.
Greetings @syasonik , Hope you are doing great. Related issue #351387 (closed) has been merged, I hope we can run pipeline for same..
Thanks for the ping @anshulriyal! Like this MR, #351387 (closed) unfortunately shouldn't have merged until %15.2. So this MR will still need to wait a bit.
Hey @anshulriyal! It's about time that we can polish this MR to be merged after the 22nd of the month. (And probably to re-add the changes from !87178 (merged) in another MR as well!
)Just a couple things that need to be addressed:
- Merge conflicts!
- Looks like we need to regenerate translations for some strings. Take a look at the static analysis failure in the pipeline for more details.
Otherwise, this is looking great!
Hi @syasonik i have created a new MR (!90532 (merged)) in place of(!87178 (merged)).
Quick question, i tried to checkout from previous branch and rebasing from master to have latest changes. however, after
rebase
all changes getting vanished. Am i doing something wrong there?I'm not totally sure @anshulriyal
I'd guess the issue is that your commits are present in master already, so the rebased branch essentially matches master. You could try cherry-picking the commits instead, so they'll end up with a different SHA?
Thank you @syasonik
I have made the desired changes. Hope pipeline workout smoothly
mentioned in issue gitlab-org/quality/triage-reports#7523 (closed)
added devopsmonitor grouprespond sectionops typemaintenance labels
changed milestone to %15.2
added maintenanceremoval typefeature labels and removed typemaintenance label
mentioned in commit anshulriyal/gitlab@31d46c70
added 1 commit
- 31d46c70 - Tracing: Remove UI at Settings > Monitor > Tracing
mentioned in commit anshulriyal/gitlab@70c8d93a
added 1 commit
- 70c8d93a - Tracing: Remove UI at Settings > Monitor > Tracing
Suggested Reviewers (beta)
The individuals below may be good candidates to participate in the review based on various factors.
You can use slash commands in comments to quickly assign
/assign_reviewer @user1
.Suggested Reviewers @ashmckenzie
,@rymai
,@kushalpandya
,@rpereira2
,@splattael
If you do not believe these suggestions are useful, please apply the label Bad Suggested Reviewer. You can also provide feedback for this feature on this issue:
https://gitlab.com/gitlab-org/gitlab/-/issues/357923
.Automatically generated by Suggested Reviewers Bot - an experimental ML-based recommendation engine created by ~"group::applied ml".
Edited by GitLab Reviewer-Recommender Bot1 Message CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, add the
Changelog
trailer to the commit message you want to add to the changelog.If you want to create a changelog entry for GitLab EE, also add the
EE: true
trailer to your commit message.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
If needed, you can retry the
danger-review
job that generated this comment.Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Krasimir Angelov ( @krasio
) (UTC+12)Marc Shaw ( @marc_shaw
) (UTC+12)frontend Coung Ngo ( @cngo
) (UTC+1)Olena HK. ( @ohoral
) (UTC+0)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Generated by
DangerLooking at this MR it seems from the conversations this is still in full-steam-ahead development mode. We recently introduced a new review workflow that makes it easy to indicate if you (and other people who collaborated here) are waiting on a review, and in effect, mark this MR as ready for another set of eyes to look at.
If you think everything is ready for this next step, please run the command
@gitlab-bot ready
. Until further notice I've set your MR to workflowin dev to make it more clear you (and others) are still working on it.Thank you so much for your hard work so far!
added workflowin dev label
@anshulriyal - Please let us know if you need help. I just saw this MR is helping close the related issue from the epic
@anshulriyal - I added a couple of comments. Please take a look. Otherwise the changes look good.
mentioned in commit anshulriyal/gitlab@278ee95f
added 6505 commits
-
70c8d93a...14add7bc - 6504 commits from branch
gitlab-org:master
- 278ee95f - Tracing: Remove UI at Settings > Monitor > Tracing
-
70c8d93a...14add7bc - 6504 commits from branch
requested review from @rkadam3
Allure report
allure-report-publisher
generated test report!review-qa-blocking:
test report for 278ee95fexpand test summary
+---------------------------------------------------------------------------+ | suites summary | +----------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +----------------------+--------+--------+---------+-------+-------+--------+ | Manage | 37 | 0 | 2 | 0 | 39 | ✅ | | Secure | 6 | 0 | 0 | 0 | 6 | ✅ | | Plan | 48 | 0 | 1 | 0 | 49 | ✅ | | Create | 23 | 0 | 2 | 0 | 25 | ✅ | | Verify | 12 | 0 | 1 | 0 | 13 | ✅ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Protect | 2 | 0 | 0 | 0 | 2 | ✅ | | Configure | 0 | 0 | 1 | 0 | 1 | ➖ | | Version sanity check | 0 | 0 | 1 | 0 | 1 | ➖ | +----------------------+--------+--------+---------+-------+-------+--------+ | Total | 128 | 0 | 9 | 0 | 137 | ✅ | +----------------------+--------+--------+---------+-------+-------+--------+
10 10 create(:project_error_tracking_setting, project: project) 11 11 end 12 12 13 13 let_it_be_with_reload(:tracing_setting) do @anshulriyal - We also do not need this declaration.
25 25 26 26 allow(view).to receive(:error_tracking_setting) 27 27 .and_return(error_tracking_setting) 28 28 allow(view).to receive(:tracing_setting) Hi @rkadam3 Thank you for your valuable time & suggestion
I'll make the desire changes and push those soon. This MR will merge after milestones 15.2 right?Hey @anshulriyal, we intend to merge this in %15.2 which is before July 16th.
With that being said, merging this MR would help me to unblock myself for other MRs in the removal
Hi, I think the changes from this MR got covered in !90567 (merged) due to inter linked dependencies that caused pipeline failures in !90567 (merged)
Sorry to stamp over
@anshulriyal - Please let me know if you would like another issue to work on as we can close this due to changes being covered in !90567 (merged)
It's heartbreaking @rkadam3 , should i close this MR🫣
@anshulriyal - I think so, yes
mentioned in issue #351387 (closed)
mentioned in issue #351388 (closed)
removed review request for @rkadam3
@anshulriyal, how was your code review experience with this merge request? Please tell us how we can continue to iterate and improve:
- Leave a
or a on this comment to describe your experience. - Create a new comment starting with
@gitlab-bot feedback
below, and leave any additional feedback you have for us in the comment.
Have five minutes? Take our survey to give us even more feedback on how GitLab can improve the contributor experience.
Thanks for your help!
- Leave a