-
Notifications
You must be signed in to change notification settings - Fork 483
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
Add View Option for raw/unadjusted trace #153
Conversation
Signed-off-by: Yuri Shkuro <ys@uber.com>
@@ -104,7 +104,12 @@ export default function TracePageHeader(props: TracePageHeaderProps) { | |||
<Dropdown.Menu> | |||
<Dropdown.Item> | |||
<a rel="noopener noreferrer" target="_blank" href={`/api/traces/${traceID}`}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yurishkuro Yes, this needs the prefixUrl(...)
.
Also, is the trailing /
in the raw version URL intentional?
/api/traces/${traceID}/?raw=true
Seems like the only difference should be the query string is appended?
/api/traces/${traceID}?raw=true
Codecov Report
@@ Coverage Diff @@
## master #153 +/- ##
=======================================
Coverage 92.66% 92.66%
=======================================
Files 85 85
Lines 1881 1881
Branches 366 366
=======================================
Hits 1743 1743
Misses 126 126
Partials 12 12
Continue to review full report at Codecov.
|
add this to the change log so it can go out with 1.1? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting change of raw URL to not the trailing slash .../${traceID}/
or confirmation that it is intentional.
Signed-off-by: Joe Farro <joef@uber.com>
Signed-off-by: Joe Farro <joef@uber.com>
@yurishkuro The I went ahead and added it (and fixed a failing unit test). |
* Add View Option for raw/unadjusted trace Signed-off-by: Yuri Shkuro <ys@uber.com> * Remove / Signed-off-by: Yuri Shkuro <ys@uber.com> * Add missing prefixUrl adjustment Signed-off-by: Joe Farro <joef@uber.com> * Fix failing unit test Signed-off-by: Joe Farro <joef@uber.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Fixes #152