Record tags inconditionally at server receive

[jcarres-mdsol]

This removes the possibility of constraining the tags recorded at server receive. It was always half-baked and pretty useless.

We originally created the feature of optionally setting tags in server because:

• UI bugs
• Being cheap with storage

I do not think any of these hold anymore.

Also, by our own experience using zipkin more often than not we want more info not less.

This PR also adds status_code and method alongside the path info to server spans so that info is available when for some reason the client spans do not have it.

@adriancole @ykitamura-mdsol @jfeltesse-mdsol

[ykitamura-mdsol]

you can remove :record_on_server_receive on line 11

[codefromthecrypt]

you can consider doubling this as error so the span appears red

ex from java

    // 1xx, 2xx, and 3xx codes are not all valid, but the math is good enough vs drift and opinion
    // about individual codes in the range.
    if (httpStatusInt < 100 || httpStatusInt > 399) {
      customizer.tag("error", String.valueOf(httpStatusInt));
    }

[ykitamura-mdsol]

Here is how we are doing it in client spans:

zipkin-ruby/lib/zipkin-tracer/faraday/zipkin-tracer.rb

Line 75 in cc4e358

span.record_tag(Trace::Span::Tag::ERROR, status) if STATUS_ERROR_REGEXP.match(status)

Maybe we can have a method like this in the Span class:

    STATUS_ERROR_REGEXP = /\A(4.*|5.*)\z/.freeze

    def record_status(status)
      record_tag(Tag::STATUS, status)
      record_tag(Tag::ERROR, status) if STATUS_ERROR_REGEXP.match(status)
    end

[jcarres-mdsol]

Done

[codefromthecrypt]

shibbit

…

On Sat, Mar 30, 2019, 6:16 AM Jordi Polo Carres ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In lib/zipkin-tracer/rack/zipkin-tracer.rb <#144 (comment)> : > annotate_plugin(span, zipkin_env.env, status, headers, body) end - def trace_request_information(span, zipkin_env) - tags = if ***@***.***_on_server_receive.empty? - # if the user specified tags to record on server receive, use these no matter what - @config.record_on_server_receive - elsif !zipkin_env.called_with_zipkin_headers? - # if the request comes from a non zipkin-enabled source record the default tags - DEFAULT_SERVER_RECV_TAGS - end - return if tags.nil? - tags.each { |annotation_key, env_key| span.record_tag(annotation_key, zipkin_env.env[env_key]) } + def trace_server_information(span, zipkin_env, status) + span.kind = Trace::Span::Kind::SERVER + span.record_tag(Trace::Span::Tag::STATUS, status) Done — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#144 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD616TvJPciS8O5a39D2lCs4ge9F590ks5vbp7JgaJpZM4cRihJ> .