-
Notifications
You must be signed in to change notification settings - Fork 24
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
Display serviceName based on service tag, if available #435
Conversation
…ace results table "service names" list
@@ -97,6 +96,8 @@ export default class Span extends React.Component { | |||
operationName | |||
} = span; | |||
|
|||
const serviceName = Span.getTagValue(span, 'service') || span.serviceName; |
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.
interesting you are prioritizing tag over field.. is this common or a bug?
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.
interesting you are prioritizing tag over field.. is this common or a bug?
This change is to visualize our indexer change (ExpediaDotCom/haystack-traces#183 in response to opentracing/specification#131). Until there's a consensus on the OT specification, we're going to override the visualized service name with service tag, if it exists
opentracing is an api not a data model specification. wouldn't you map it
to service name instead of handling here? I dont see haystack participating
but somehow willing to just take whatever they randomly think up.
…On Wed, 28 Nov 2018, 09:32 Jason Bulicek ***@***.*** wrote:
***@***.**** commented on this pull request.
------------------------------
In src/components/traces/details/timeline/span.jsx
<#435 (comment)>
:
> @@ -97,6 +96,8 @@ export default class Span extends React.Component {
operationName
} = span;
+ const serviceName = Span.getTagValue(span, 'service') || span.serviceName;
interesting you are prioritizing tag over field.. is this common or a bug?
This change is to visualize our indexer change (
ExpediaDotCom/haystack-traces#183
<ExpediaDotCom/haystack-traces#183> in response
to opentracing/specification#131
<opentracing/specification#131>). Until there's
a consensus on the OT specification, we're going to override the visualized
service name with service tag, if it exists
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#435 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAD617A2dvF71HG9aAHnu0N_g7IuYY9Nks5uzb2SgaJpZM4Y2WrP>
.
|
Adrian, we were uncomfortable changing the raw data. We have an internal client who needs to "override" service name and this recent change to the spec fit our use case nicely. You have correctly pointed out the problems with this approach, and we fully expect to honor the spec when it changes. |
question is Jeff is do you expect it to change for the better if you don't
tell opentracing leadership your opinion? my challenge for you is to place
your opinion on the issue else like many other things I would expect this
OT tech debt to persist.
sites usually never speak up in other words except uber. almost always
vendors. you can voice your opinion in a way more likely to be heard than
mine.
…On Wed, 28 Nov 2018, 09:50 Jeff Baker ***@***.*** wrote:
opentracing is an api not a data model specification. wouldn't you map it
to service name instead of handling here? I dont see haystack participating
but somehow willing to just take whatever they randomly think up.
… <#m_-5926400379466294230_>
On Wed, 28 Nov 2018, 09:32 Jason Bulicek ***@***.*** wrote: ***@***.****
commented on this pull request. ------------------------------ In
src/components/traces/details/timeline/span.jsx <#435 (comment)
<#435 (comment)>>
: > @@ -97,6 +96,8 @@ export default class Span extends React.Component {
operationName } = span; + const serviceName = Span.getTagValue(span,
'service') || span.serviceName; interesting you are prioritizing tag over
field.. is this common or a bug? This change is to visualize our indexer
change ( ExpediaDotCom/haystack-traces#183
<ExpediaDotCom/haystack-traces#183> <
ExpediaDotCom/haystack-traces#183
<ExpediaDotCom/haystack-traces#183>> in response
to opentracing/specification#131
<opentracing/specification#131> <
opentracing/specification#131
<opentracing/specification#131>>). Until
there's a consensus on the OT specification, we're going to override the
visualized service name with service tag, if it exists — You are receiving
this because you commented. Reply to this email directly, view it on GitHub
<#435 (comment)
<#435 (comment)>>,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAD617A2dvF71HG9aAHnu0N_g7IuYY9Nks5uzb2SgaJpZM4Y2WrP
.
Adrian, we were uncomfortable changing the raw data. We have an internal
client who needs to "override" service name and this recent change to the
spec fit our use case nicely. You have correctly pointed out the problems
with this approach, and we fully expect to honor the spec when it changes.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#435 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAD61_oyC7ObPXwyHl0Oplx3fCmne_a4ks5uzcHLgaJpZM4Y2WrP>
.
|
No description provided.