From 4ff2a58625829367b00e0afd5e995561edb68b35 Mon Sep 17 00:00:00 2001 From: Yohei Kitamura Date: Sat, 9 Mar 2019 03:07:58 +0900 Subject: [PATCH] Switch to Zipkin v2 span format (#141) * Switch to Zipkin v2 span format * Replace 'http.status' with 'http.status_code' * Add shared * Add todo to LOCAL_COMPONENT --- CHANGELOG.md | 3 + README.md | 6 +- lib/zipkin-tracer/excon/zipkin-tracer.rb | 28 ++-- lib/zipkin-tracer/faraday/zipkin-tracer.rb | 33 ++--- lib/zipkin-tracer/hostname_resolver.rb | 25 ++-- lib/zipkin-tracer/rack/zipkin-tracer.rb | 5 +- lib/zipkin-tracer/rack/zipkin_env.rb | 8 +- lib/zipkin-tracer/trace.rb | 92 ++++++------- lib/zipkin-tracer/version.rb | 2 +- lib/zipkin-tracer/zipkin_json_tracer.rb | 2 +- lib/zipkin-tracer/zipkin_tracer_base.rb | 7 +- spec/lib/excon/zipkin-tracer_spec.rb | 28 ++-- spec/lib/hostname_resolver_spec.rb | 21 +-- spec/lib/middleware_shared_examples.rb | 38 ++---- spec/lib/rack/zipkin-tracer_spec.rb | 13 +- spec/lib/rack/zipkin_env_spec.rb | 8 ++ spec/lib/trace_spec.rb | 151 ++++++++++++--------- spec/lib/zipkin_tracer_base_spec.rb | 34 +++-- 18 files changed, 238 insertions(+), 266 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7c3c03..b84e878 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +# 0.33.0 +* Switch to Zipkin v2 span format + # 0.32.4 * Remove the ':service_port' configuration. * Fix 'sa' annotation encoding. diff --git a/README.md b/README.md index 3031e8d..218cf3e 100644 --- a/README.md +++ b/README.md @@ -84,7 +84,7 @@ It can be used to measure the performance of process, record value of variables, When `local_component_span` method is called, it creates a new span and a local component, and provides the following methods to create annotations. * record(key) - annotation -* record_tag(key, value) - binary annotation +* record_tag(key, value) - tag Example: ```ruby @@ -105,7 +105,7 @@ Only one of the following tracers can be used at a given time. Sends traces as JSON over HTTP. This is the preferred tracer to use as the openzipkin project moves away from Thrift. -You need to specify the `:json_api_host` parameter to wherever your zipkin collector is running. It will POST traces to the `/api/v1/spans` path. +You need to specify the `:json_api_host` parameter to wherever your zipkin collector is running. It will POST traces to the `/api/v2/spans` path. ### Kafka @@ -164,7 +164,7 @@ lambda do |span, env, status, response_headers, response_body| span.record_tag('http.referrer', env['HTTP_REFERRER']) # integer annotation span.record_tag('http.content_size', env['CONTENT_SIZE'].to_s) - span.record_tag('http.status', status) + span.record_tag('http.status_code', status) end ``` diff --git a/lib/zipkin-tracer/excon/zipkin-tracer.rb b/lib/zipkin-tracer/excon/zipkin-tracer.rb index 57c8499..9bbb405 100644 --- a/lib/zipkin-tracer/excon/zipkin-tracer.rb +++ b/lib/zipkin-tracer/excon/zipkin-tracer.rb @@ -28,10 +28,7 @@ def request_call(datum) def response_call(datum) if span = datum[:span] status = response_status(datum) - if status - record_response_tags(span, status, local_endpoint) - end - span.record(Trace::Annotation::CLIENT_RECV, local_endpoint) + record_response_tags(span, status) if status Trace.tracer.end_span(span) end @@ -53,12 +50,8 @@ def b3_headers } end - def local_endpoint - Trace.default_endpoint # The rack middleware set this up for us. - end - def remote_endpoint(url, service_name) - Trace::Endpoint.remote_endpoint(url, service_name, local_endpoint.ip_format) # The endpoint we are calling. + Trace::Endpoint.remote_endpoint(url, service_name, Trace.default_endpoint.ip_format) # The endpoint we are calling. end def service_name(datum, default) @@ -69,12 +62,9 @@ def response_status(datum) datum[:response] && datum[:response][:status] && datum[:response][:status].to_s end - def record_response_tags(span, status, local_endpoint) - span.record_tag(Trace::BinaryAnnotation::STATUS, status, Trace::BinaryAnnotation::Type::STRING, local_endpoint) - if STATUS_ERROR_REGEXP.match(status) - span.record_tag(Trace::BinaryAnnotation::ERROR, status, - Trace::BinaryAnnotation::Type::STRING, local_endpoint) - end + def record_response_tags(span, status) + span.record_tag(Trace::Span::Tag::STATUS, status) + span.record_tag(Trace::Span::Tag::ERROR, status) if STATUS_ERROR_REGEXP.match(status) end def trace!(datum, trace_id) @@ -85,10 +75,10 @@ def trace!(datum, trace_id) span = Trace.tracer.start_span(trace_id, method.downcase) # annotate with method (GET/POST/etc.) and uri path - span.record_tag(Trace::BinaryAnnotation::METHOD, method.upcase, Trace::BinaryAnnotation::Type::STRING, local_endpoint) - span.record_tag(Trace::BinaryAnnotation::PATH, url.path, Trace::BinaryAnnotation::Type::STRING, local_endpoint) - span.record_tag(Trace::BinaryAnnotation::SERVER_ADDRESS, SERVER_ADDRESS_SPECIAL_VALUE, Trace::BinaryAnnotation::Type::BOOL, remote_endpoint(url, service_name)) - span.record(Trace::Annotation::CLIENT_SEND, local_endpoint) + span.kind = Trace::Span::Kind::CLIENT + span.remote_endpoint = remote_endpoint(url, service_name) + span.record_tag(Trace::Span::Tag::METHOD, method.upcase) + span.record_tag(Trace::Span::Tag::PATH, url.path) # store the span in the datum hash so it can be used in the response_call datum[:span] = span diff --git a/lib/zipkin-tracer/faraday/zipkin-tracer.rb b/lib/zipkin-tracer/faraday/zipkin-tracer.rb index e1a4b46..caa8cc9 100644 --- a/lib/zipkin-tracer/faraday/zipkin-tracer.rb +++ b/lib/zipkin-tracer/faraday/zipkin-tracer.rb @@ -44,42 +44,37 @@ def trace!(env, trace_id) # handle either a URI object (passed by Faraday v0.8.x in testing), or something string-izable method = env[:method].to_s url = env[:url].respond_to?(:host) ? env[:url] : URI.parse(env[:url].to_s) - local_endpoint = Trace.default_endpoint # The rack middleware set this up for us. - remote_endpoint = Trace::Endpoint.remote_endpoint(url, @service_name, local_endpoint.ip_format) # The endpoint we are calling. + remote_endpoint = Trace::Endpoint.remote_endpoint(url, @service_name, Trace.default_endpoint.ip_format) # The endpoint we are calling. Trace.tracer.with_new_span(trace_id, method.downcase) do |span| @span = span # So we can record on exceptions # annotate with method (GET/POST/etc.) and uri path - span.record_tag(Trace::BinaryAnnotation::METHOD, method.upcase, Trace::BinaryAnnotation::Type::STRING, local_endpoint) - span.record_tag(Trace::BinaryAnnotation::PATH, url.path, Trace::BinaryAnnotation::Type::STRING, local_endpoint) - span.record_tag(Trace::BinaryAnnotation::SERVER_ADDRESS, SERVER_ADDRESS_SPECIAL_VALUE, Trace::BinaryAnnotation::Type::BOOL, remote_endpoint) - span.record(Trace::Annotation::CLIENT_SEND, local_endpoint) + span.kind = Trace::Span::Kind::CLIENT + span.remote_endpoint = remote_endpoint + span.record_tag(Trace::Span::Tag::METHOD, method.upcase) + span.record_tag(Trace::Span::Tag::PATH, url.path) response = @app.call(env).on_complete do |renv| - record_response_tags(span, renv[:status].to_s, local_endpoint) + record_response_tags(span, renv[:status].to_s) end - span.record(Trace::Annotation::CLIENT_RECV, local_endpoint) end response rescue Net::ReadTimeout - record_error(@span, 'Request timed out.', local_endpoint) + record_error(@span, 'Request timed out.') raise rescue Faraday::ConnectionFailed - record_error(@span, 'Request connection failed.', local_endpoint) + record_error(@span, 'Request connection failed.') raise rescue Faraday::ClientError - record_error(@span, 'Generic Faraday client error.', local_endpoint) + record_error(@span, 'Generic Faraday client error.') raise end - def record_error(span, msg, local_endpoint) - span.record_tag(Trace::BinaryAnnotation::ERROR, msg, Trace::BinaryAnnotation::Type::STRING, local_endpoint) + def record_error(span, msg) + span.record_tag(Trace::Span::Tag::ERROR, msg) end - def record_response_tags(span, status, local_endpoint) - span.record_tag(Trace::BinaryAnnotation::STATUS, status, Trace::BinaryAnnotation::Type::STRING, local_endpoint) - if STATUS_ERROR_REGEXP.match(status) - span.record_tag(Trace::BinaryAnnotation::ERROR, status, - Trace::BinaryAnnotation::Type::STRING, local_endpoint) - end + def record_response_tags(span, status) + span.record_tag(Trace::Span::Tag::STATUS, status) + span.record_tag(Trace::Span::Tag::ERROR, status) if STATUS_ERROR_REGEXP.match(status) end end diff --git a/lib/zipkin-tracer/hostname_resolver.rb b/lib/zipkin-tracer/hostname_resolver.rb index 16019cd..e00ebd3 100644 --- a/lib/zipkin-tracer/hostname_resolver.rb +++ b/lib/zipkin-tracer/hostname_resolver.rb @@ -1,17 +1,17 @@ require 'resolv' module ZipkinTracer - # Resolves hostnames in the endpoints of the annotations. + # Resolves hostnames in the endpoints of the spans. # Resolving hostnames is a very expensive operation. We want to store them raw in the main thread # and resolve them in a different thread where we do not affect execution times. class HostnameResolver def spans_with_ips(spans) host_to_ip = hosts_to_ipv4(spans) - each_annotation(spans) do |annotation| - hostname = annotation.host.ipv4 + each_endpoint(spans) do |endpoint| + hostname = endpoint.ipv4 unless resolved_ip_address?(hostname.to_s) - annotation.host.ipv4 = host_to_ip[hostname] + endpoint.ipv4 = host_to_ip[hostname] end end end @@ -28,24 +28,19 @@ def resolved_ip_address?(ip_string) ip_string.to_i.to_s == ip_string end - def each_annotation(spans, &block) + def each_endpoint(spans, &block) spans.each do |span| - span.annotations.each do |annotation| - yield annotation - end - span.binary_annotations.each do |annotation| - yield annotation + [span.local_endpoint, span.remote_endpoint].each do |endpoint| + yield endpoint if endpoint end end end - # Annotations come in pairs like CS/CR, SS/SR. - # Each annnotation has a hostname so we, for sure, will have the same host multiple times. # Using this to resolve only once per host def hosts_to_ipv4(spans) hosts = [] - each_annotation(spans) do |annotation| - hosts.push(annotation.host) + each_endpoint(spans) do |endpoint| + hosts.push(endpoint) end hosts.uniq! resolve(hosts) @@ -61,7 +56,7 @@ def resolve(hosts) end def host_to_ip(hostname, ip_format) - ipv4 = begin + begin ip_format == :string ? Socket.getaddrinfo(hostname, nil, :INET).first[IP_FIELD] : Trace::Endpoint.host_to_i32(hostname) rescue ip_format == :string ? LOCALHOST : LOCALHOST_I32 diff --git a/lib/zipkin-tracer/rack/zipkin-tracer.rb b/lib/zipkin-tracer/rack/zipkin-tracer.rb index 8d919a8..3ac5d5b 100644 --- a/lib/zipkin-tracer/rack/zipkin-tracer.rb +++ b/lib/zipkin-tracer/rack/zipkin-tracer.rb @@ -12,7 +12,7 @@ class RackHandler REQUEST_METHOD = Rack::REQUEST_METHOD rescue 'REQUEST_METHOD'.freeze DEFAULT_SERVER_RECV_TAGS = { - Trace::BinaryAnnotation::PATH => PATH_INFO + Trace::Span::Tag::PATH => PATH_INFO }.freeze def initialize(app, config = nil) @@ -55,12 +55,11 @@ def annotate_plugin(span, env, status, response_headers, response_body) def trace!(span, zipkin_env, &block) trace_request_information(span, zipkin_env) - span.record(Trace::Annotation::SERVER_RECV) + span.kind = Trace::Span::Kind::SERVER span.record('whitelisted') if zipkin_env.force_sample? status, headers, body = yield ensure annotate_plugin(span, zipkin_env.env, status, headers, body) - span.record(Trace::Annotation::SERVER_SEND) end def trace_request_information(span, zipkin_env) diff --git a/lib/zipkin-tracer/rack/zipkin_env.rb b/lib/zipkin-tracer/rack/zipkin_env.rb index 71ad095..f418b89 100644 --- a/lib/zipkin-tracer/rack/zipkin_env.rb +++ b/lib/zipkin-tracer/rack/zipkin_env.rb @@ -10,10 +10,10 @@ def initialize(env, config) end def trace_id(default_flags = Trace::Flags::EMPTY) - trace_id, span_id, parent_span_id = retrieve_or_generate_ids + trace_id, span_id, parent_span_id, shared = retrieve_or_generate_ids sampled = sampled_header_value(@env['HTTP_X_B3_SAMPLED']) flags = (@env['HTTP_X_B3_FLAGS'] || default_flags).to_i - Trace::TraceId.new(trace_id, parent_span_id, span_id, sampled, flags) + Trace::TraceId.new(trace_id, parent_span_id, span_id, sampled, flags, shared) end def called_with_zipkin_headers? @@ -33,12 +33,14 @@ def retrieve_or_generate_ids if called_with_zipkin_headers? trace_id, span_id = @env.values_at(*B3_REQUIRED_HEADERS) parent_span_id = @env['HTTP_X_B3_PARENTSPANID'] + shared = true else span_id = TraceGenerator.new.generate_id trace_id = TraceGenerator.new.generate_id_from_span_id(span_id) parent_span_id = nil + shared = false end - [trace_id, span_id, parent_span_id] + [trace_id, span_id, parent_span_id, shared] end def new_sampled_header_value(sampled) diff --git a/lib/zipkin-tracer/trace.rb b/lib/zipkin-tracer/trace.rb index 5d54e34..def4679 100644 --- a/lib/zipkin-tracer/trace.rb +++ b/lib/zipkin-tracer/trace.rb @@ -48,54 +48,17 @@ def default_endpoint # Moved here as a first step, eventually move them out of the Trace module class Annotation - CLIENT_SEND = "cs" - CLIENT_RECV = "cr" - SERVER_SEND = "ss" - SERVER_RECV = "sr" + attr_reader :value, :timestamp - attr_reader :value, :host, :timestamp - def initialize(value, host) + def initialize(value) @timestamp = (Time.now.to_f * 1000 * 1000).to_i # micros @value = value - @host = host - end - - def to_h - { - value: @value, - timestamp: @timestamp, - endpoint: host.to_h - } - end - end - - class BinaryAnnotation - SERVER_ADDRESS = 'sa'.freeze - URI = 'http.url'.freeze - METHOD = 'http.method'.freeze - PATH = 'http.path'.freeze - STATUS = 'http.status'.freeze - LOCAL_COMPONENT = 'lc'.freeze - ERROR = 'error'.freeze - - module Type - BOOL = "BOOL" - STRING = "STRING" - end - attr_reader :key, :value, :host - - def initialize(key, value, annotation_type, host) - @key = key - @value = value - @annotation_type = annotation_type - @host = host end def to_h { - key: @key, value: @value, - endpoint: host.to_h + timestamp: @timestamp } end end @@ -138,14 +101,15 @@ def to_i; @i64; end # A TraceId contains all the information of a given trace id class TraceId - attr_reader :trace_id, :parent_id, :span_id, :sampled, :flags + attr_reader :trace_id, :parent_id, :span_id, :sampled, :flags, :shared - def initialize(trace_id, parent_id, span_id, sampled, flags) + def initialize(trace_id, parent_id, span_id, sampled, flags, shared = false) @trace_id = Trace.trace_id_128bit ? TraceId128Bit.from_value(trace_id) : SpanId.from_value(trace_id) @parent_id = parent_id.nil? ? nil : SpanId.from_value(parent_id) @span_id = SpanId.from_value(span_id) @sampled = sampled @flags = flags + @shared = shared end def next_id @@ -162,7 +126,8 @@ def sampled? end def to_s - "TraceId(trace_id = #{@trace_id.to_s}, parent_id = #{@parent_id.to_s}, span_id = #{@span_id.to_s}, sampled = #{@sampled.to_s}, flags = #{@flags.to_s})" + "TraceId(trace_id = #{@trace_id.to_s}, parent_id = #{@parent_id.to_s}, span_id = #{@span_id.to_s}," \ + " sampled = #{@sampled.to_s}, flags = #{@flags.to_s}, shared = #{@shared.to_s})" end end @@ -199,12 +164,29 @@ def to_i; @i128; end # A span may contain many annotations class Span - attr_accessor :name, :annotations, :binary_annotations, :debug + module Tag + METHOD = "http.method".freeze + PATH = "http.path".freeze + STATUS = "http.status_code".freeze + LOCAL_COMPONENT = "lc".freeze # TODO: Remove LOCAL_COMPONENT and related methods when no longer needed + ERROR = "error".freeze + end + + module Kind + CLIENT = "CLIENT".freeze + SERVER = "SERVER".freeze + end + + attr_accessor :name, :kind, :local_endpoint, :remote_endpoint, :annotations, :tags, :debug + def initialize(name, span_id) @name = name @span_id = span_id + @kind = nil + @local_endpoint = nil + @remote_endpoint = nil @annotations = [] - @binary_annotations = [] + @tags = {} @debug = span_id.debug? @timestamp = to_microseconds(Time.now) @duration = UNKNOWN_DURATION @@ -219,28 +201,31 @@ def to_h name: @name, traceId: @span_id.trace_id.to_s, id: @span_id.span_id.to_s, - annotations: @annotations.map(&:to_h), - binaryAnnotations: @binary_annotations.map(&:to_h), + localEndpoint: @local_endpoint.to_h, timestamp: @timestamp, duration: @duration, debug: @debug } h[:parentId] = @span_id.parent_id.to_s unless @span_id.parent_id.nil? + h[:kind] = @kind unless @kind.nil? + h[:remoteEndpoint] = @remote_endpoint.to_h unless @remote_endpoint.nil? + h[:annotations] = @annotations.map(&:to_h) unless @annotations.empty? + h[:tags] = @tags unless @tags.empty? + h[:shared] = true if @span_id.shared h end # We record information into spans, then we send these spans to zipkin - def record(value, endpoint = Trace.default_endpoint) - annotations << Trace::Annotation.new(value.to_s, endpoint) + def record(value) + annotations << Trace::Annotation.new(value.to_s) end - def record_tag(key, value, type = Trace::BinaryAnnotation::Type::STRING, endpoint = Trace.default_endpoint) - value = value.to_s if type == Trace::BinaryAnnotation::Type::STRING - binary_annotations << Trace::BinaryAnnotation.new(key, value, type, endpoint) + def record_tag(key, value) + @tags[key] = value end def record_local_component(value) - record_tag(BinaryAnnotation::LOCAL_COMPONENT, value) + record_tag(Tag::LOCAL_COMPONENT, value) end def has_parent_span? @@ -293,6 +278,5 @@ def to_h hsh[:port] = port if port hsh end - end end diff --git a/lib/zipkin-tracer/version.rb b/lib/zipkin-tracer/version.rb index 83df7be..ae6f8ca 100644 --- a/lib/zipkin-tracer/version.rb +++ b/lib/zipkin-tracer/version.rb @@ -1,3 +1,3 @@ module ZipkinTracer - VERSION = '0.32.4'.freeze + VERSION = '0.33.0'.freeze end diff --git a/lib/zipkin-tracer/zipkin_json_tracer.rb b/lib/zipkin-tracer/zipkin_json_tracer.rb index 1c91da9..0d822b3 100644 --- a/lib/zipkin-tracer/zipkin_json_tracer.rb +++ b/lib/zipkin-tracer/zipkin_json_tracer.rb @@ -5,7 +5,7 @@ class AsyncJsonApiClient include SuckerPunch::Job - SPANS_PATH = '/api/v1/spans' + SPANS_PATH = '/api/v2/spans' def perform(json_api_host, spans) spans_with_ips = ::ZipkinTracer::HostnameResolver.new.spans_with_ips(spans).map(&:to_h) diff --git a/lib/zipkin-tracer/zipkin_tracer_base.rb b/lib/zipkin-tracer/zipkin_tracer_base.rb index 308c96d..9171939 100644 --- a/lib/zipkin-tracer/zipkin_tracer_base.rb +++ b/lib/zipkin-tracer/zipkin_tracer_base.rb @@ -22,12 +22,10 @@ def with_new_span(trace_id, name) def end_span(span) span.close - # If in a thread not handling incoming http requests, it will not have Annotation::SERVER_SEND, so the span + # If in a thread not handling incoming http requests, it will not have Kind::SERVER, so the span # will never be flushed and will cause memory leak. - # It will have CLIENT_SEND and CLIENT_RECV if the thread sends out http requests, so use CLIENT_RECV as the sign - # to flush the span. # If no parent span, then current span needs to flush when it ends. - if !span.has_parent_span? || span.annotations.any? { |ann| ann.value == Annotation::SERVER_SEND } + if !span.has_parent_span? || span.kind == Trace::Span::Kind::SERVER flush! reset end @@ -35,6 +33,7 @@ def end_span(span) def start_span(trace_id, name) span = Span.new(name, trace_id) + span.local_endpoint = Trace.default_endpoint store_span(trace_id, span) span end diff --git a/spec/lib/excon/zipkin-tracer_spec.rb b/spec/lib/excon/zipkin-tracer_spec.rb index cb3f499..2a768f3 100644 --- a/spec/lib/excon/zipkin-tracer_spec.rb +++ b/spec/lib/excon/zipkin-tracer_spec.rb @@ -135,12 +135,11 @@ def process(body, url, headers = {}) span = spy('Trace::Span') allow(Trace::Span).to receive(:new).and_return(span) + expect(span).to receive(:record_tag).with("http.path", "/some/path/here") + ZipkinTracer::TraceContainer.with_trace_id(trace_id) do connection.request end - - expect(span).to have_received(:record_tag) - .with("http.path", "/some/path/here", "STRING", anything) end end @@ -162,12 +161,11 @@ def process(body, url, headers = {}) span = spy('Trace::Span') allow(Trace::Span).to receive(:new).and_return(span) + expect(span).to receive(:record_tag).with("http.path", "/some/path/here") + ZipkinTracer::TraceContainer.with_trace_id(trace_id) do connection.request(path: url_path, query: { query: "params" }) end - - expect(span).to have_received(:record_tag) - .with("http.path", "/some/path/here", "STRING", anything) end end end @@ -210,13 +208,9 @@ def process(body, url, headers = {}) span = spy('Trace::Span') allow(Trace::Span).to receive(:new).and_return(span) - expect(span).to receive(:record_tag) do |name, _, _, host| - if name == Trace::BinaryAnnotation::SERVER_ADDRESS - expect(host.service_name).to eql("fake-service-name") - else - true - end - end.at_least(:once) + expect(span).to receive(:remote_endpoint=) do |host| + expect(host.service_name).to eql("fake-service-name") + end.once ZipkinTracer::TraceContainer.with_trace_id(trace_id) do connection.request @@ -230,13 +224,7 @@ def process(body, url, headers = {}) span = spy('Trace::Span') allow(Trace::Span).to receive(:new).and_return(span) - expect(span).to receive(:record_tag) do |name, value, _, _| - if name == Trace::BinaryAnnotation::STATUS - expect(value).to eql("200") - else - true - end - end.at_least(:once) + expect(span).to receive(:record_tag).with(Trace::Span::Tag::STATUS, "200").once ZipkinTracer::TraceContainer.with_trace_id(trace_id) do middlewares = [ZipkinTracer::ExconHandler] + Excon.defaults[:middlewares] diff --git a/spec/lib/hostname_resolver_spec.rb b/spec/lib/hostname_resolver_spec.rb index 915df09..5cdebcc 100644 --- a/spec/lib/hostname_resolver_spec.rb +++ b/spec/lib/hostname_resolver_spec.rb @@ -30,13 +30,13 @@ expect(resolved_spans.first).to be_kind_of(Trace::Span) end - it 'resolves the hostnames in the annotations' do - ip = resolved_spans.first.annotations.first.host.ipv4 + it 'resolves the hostnames in local_endpoint' do + ip = resolved_spans.first.local_endpoint.ipv4 expect(ip).to eq(expected_ip) end - it 'resolves the hostnames in the binnary annotations' do - ip = resolved_spans.first.binary_annotations.first.host.ipv4 + it 'resolves the hostnames in remote_endpoint' do + ip = resolved_spans.first.remote_endpoint.ipv4 expect(ip).to eq(expected_ip) end end @@ -44,7 +44,8 @@ context 'resolving to i32 addresses' do before do endpoint.ip_format = :i32 - Trace.default_endpoint = endpoint + span.local_endpoint = endpoint + span.remote_endpoint = endpoint span.record('diary') span.record_tag('secret', 'book') allow(Socket).to receive(:getaddrinfo).with(hostname, nil).and_return([[nil, nil, nil, ipv4]]) @@ -54,7 +55,7 @@ context 'host lookup failure' do before { allow(Socket).to receive(:getaddrinfo).and_raise } it 'falls back to localhost as an i32' do - ip = resolved_spans.first.annotations.first.host.ipv4 + ip = resolved_spans.first.local_endpoint.ipv4 expect(ip).to eq(0x7f000001) end end @@ -65,7 +66,8 @@ context 'with spans containing local addresses' do before do local_endpoint.ip_format = :string - Trace.default_endpoint = local_endpoint + span.local_endpoint = local_endpoint + span.remote_endpoint = local_endpoint span.record('diary') span.record_tag('secret', 'book') allow(Socket).to receive(:getaddrinfo).with(local_hostname, nil, :INET).and_return([[nil, nil, nil, ipv4]]) @@ -78,7 +80,8 @@ context 'with spans resolving to string addresses' do before do endpoint.ip_format = :string - Trace.default_endpoint = endpoint + span.local_endpoint = endpoint + span.remote_endpoint = endpoint span.record('diary') span.record_tag('secret', 'book') allow(Socket).to receive(:getaddrinfo).with(hostname, nil, :INET).and_return([[nil, nil, nil, ipv4]]) @@ -88,7 +91,7 @@ context 'host lookup failure' do before { allow(Socket).to receive(:getaddrinfo).and_raise } it 'falls back to localhost as an string' do - ip = resolved_spans.first.annotations.first.host.ipv4 + ip = resolved_spans.first.local_endpoint.ipv4 expect(ip).to eq('127.0.0.1') end end diff --git a/spec/lib/middleware_shared_examples.rb b/spec/lib/middleware_shared_examples.rb index ac7bc4f..5fa8a29 100644 --- a/spec/lib/middleware_shared_examples.rb +++ b/spec/lib/middleware_shared_examples.rb @@ -53,45 +53,30 @@ def expect_tracing expect(tracer).to receive(:start_span).with(anything, 'post').and_call_original expect(tracer).to receive(:end_span).with(anything).and_call_original - expect_any_instance_of(Trace::Span).to receive(:record_tag) do |_, key, value, type, host| + expect_any_instance_of(Trace::Span).to receive(:kind=).with('CLIENT') + + expect_any_instance_of(Trace::Span).to receive(:remote_endpoint=) do |_, host| + expect_host(host, hostname, service_name) + end + + expect_any_instance_of(Trace::Span).to receive(:record_tag) do |_, key, value| expect(key).to eq('http.method') expect(value).to eq('POST') - expect_host(host, '127.0.0.1', service_name) end - expect_any_instance_of(Trace::Span).to receive(:record_tag) do |_, key, value, type, host| + expect_any_instance_of(Trace::Span).to receive(:record_tag) do |_, key, value| expect(key).to eq('http.path') expect(value).to eq(url_path) - expect_host(host, '127.0.0.1', service_name) end - expect_any_instance_of(Trace::Span).to receive(:record_tag) do |_, key, value, type, host| - expect(key).to eq('sa') - expect(value).to eq(true) - expect(type).to eq('BOOL') - expect_host(host, hostname, service_name) - end - - expect_any_instance_of(Trace::Span).to receive(:record_tag) do |_, key, value, type, host| - expect(key).to eq('http.status') + expect_any_instance_of(Trace::Span).to receive(:record_tag) do |_, key, value| + expect(key).to eq('http.status_code') expect(value).to eq('404') - expect_host(host, '127.0.0.1', service_name) end - expect_any_instance_of(Trace::Span).to receive(:record_tag) do |_, key, value, type, host| + expect_any_instance_of(Trace::Span).to receive(:record_tag) do |_, key, value| expect(key).to eq('error') expect(value).to eq('404') - expect_host(host, '127.0.0.1', service_name) - end - - expect_any_instance_of(Trace::Span).to receive(:record) do |_, value, host| - expect(value).to eq(Trace::Annotation::CLIENT_SEND) - expect_host(host, '127.0.0.1', service_name) - end - - expect_any_instance_of(Trace::Span).to receive(:record) do |_, value, host| - expect(value).to eq(Trace::Annotation::CLIENT_RECV) - expect_host(host, '127.0.0.1', service_name) end end @@ -180,7 +165,6 @@ def expect_tracing end it 'does not create any annotation' do - expect(Trace::BinaryAnnotation).not_to receive(:new) expect(Trace::Annotation).not_to receive(:new) process('', url) end diff --git a/spec/lib/rack/zipkin-tracer_spec.rb b/spec/lib/rack/zipkin-tracer_spec.rb index 3ea4cf5..4c483ce 100644 --- a/spec/lib/rack/zipkin-tracer_spec.rb +++ b/spec/lib/rack/zipkin-tracer_spec.rb @@ -46,8 +46,7 @@ def expect_host(host) expect(ZipkinTracer::TraceContainer).to receive(:with_trace_id).and_call_original expect(tracer).to receive(:with_new_span).ordered.with(anything, 'get').and_call_original expect_any_instance_of(Trace::Span).to receive(:record_tag).with('http.path', '/') - expect_any_instance_of(Trace::Span).to receive(:record).with(Trace::Annotation::SERVER_RECV) - expect_any_instance_of(Trace::Span).to receive(:record).with(Trace::Annotation::SERVER_SEND) + expect_any_instance_of(Trace::Span).to receive(:kind=).with(Trace::Span::Kind::SERVER) status, headers, body = subject.call(mock_env) expect(status).to eq(app_status) @@ -98,8 +97,7 @@ def expect_host(host) expect(ZipkinTracer::TraceContainer).to receive(:with_trace_id).and_call_original expect(tracer).to receive(:with_new_span).ordered.with(anything, 'get /thing/:id').and_call_original expect_any_instance_of(Trace::Span).to receive(:record_tag).with('http.path', '/thing/123') - expect_any_instance_of(Trace::Span).to receive(:record).with(Trace::Annotation::SERVER_RECV) - expect_any_instance_of(Trace::Span).to receive(:record).with(Trace::Annotation::SERVER_SEND) + expect_any_instance_of(Trace::Span).to receive(:kind=).with(Trace::Span::Kind::SERVER) status, headers, body = subject.call(mock_env_route) expect(status).to eq(app_status) @@ -177,7 +175,7 @@ def expect_host(host) # string annotation span.record_tag('foo', env['foo'] || 'FOO') # integer annotation - span.record_tag('http.status', status) + span.record_tag('http.status_code', status) end end subject { middleware(app, annotate_plugin: annotate) } @@ -187,7 +185,7 @@ def expect_host(host) expect(tracer).to receive(:with_new_span).and_call_original.ordered expect_any_instance_of(Trace::Span).to receive(:record_tag).exactly(3).times - expect_any_instance_of(Trace::Span).to receive(:record).exactly(2).times + expect_any_instance_of(Trace::Span).to receive(:kind=).with(Trace::Span::Kind::SERVER) status, _, _ = subject.call(mock_env) # return expected status @@ -218,10 +216,9 @@ def expect_host(host) subject { middleware(app, whitelist_plugin: lambda { |env| true }, sample_rate: 0) } it 'samples the request' do + expect_any_instance_of(Trace::Span).to receive(:kind=).with(Trace::Span::Kind::SERVER) expect_any_instance_of(Trace::Span).to receive(:record_tag).with('http.path', '/') - expect_any_instance_of(Trace::Span).to receive(:record).with(Trace::Annotation::SERVER_RECV) expect_any_instance_of(Trace::Span).to receive(:record).with('whitelisted') - expect_any_instance_of(Trace::Span).to receive(:record).with(Trace::Annotation::SERVER_SEND) status, _, _ = subject.call(mock_env) expect(status).to eq(200) end diff --git a/spec/lib/rack/zipkin_env_spec.rb b/spec/lib/rack/zipkin_env_spec.rb index a283226..4ab4b5a 100644 --- a/spec/lib/rack/zipkin_env_spec.rb +++ b/spec/lib/rack/zipkin_env_spec.rb @@ -83,6 +83,10 @@ def mock_env(params = {}, path = '/') expect(zipkin_env.trace_id.sampled?).to eq(true) end + it 'shared is false' do + expect(zipkin_env.trace_id.shared).to eq(false) + end + context 'trace_id_128bit is true' do before do allow(Trace).to receive(:trace_id_128bit).and_return(true) @@ -103,6 +107,10 @@ def mock_env(params = {}, path = '/') expect(zipkin_env.called_with_zipkin_headers?).to eq(true) end + it 'shared is true' do + expect(zipkin_env.trace_id.shared).to eq(true) + end + context 'parent_id is not provided' do it 'uses the trace_id and span_id' do trace_id = zipkin_env.trace_id diff --git a/spec/lib/trace_spec.rb b/spec/lib/trace_spec.rb index 35aba40..df811cc 100644 --- a/spec/lib/trace_spec.rb +++ b/spec/lib/trace_spec.rb @@ -19,7 +19,8 @@ let(:parent_id) { 'f0e71086411b1445' } let(:sampled) { true } let(:flags) { Trace::Flags::EMPTY } - let(:trace_id) { Trace::TraceId.new(traceid, parent_id, span_id, sampled, flags) } + let(:shared) { false } + let(:trace_id) { Trace::TraceId.new(traceid, parent_id, span_id, sampled, flags, shared) } it 'is not a debug trace' do expect(trace_id.debug?).to eq(false) @@ -60,6 +61,13 @@ end end + context 'shared value is true' do + let(:shared) { true } + it 'is shared' do + expect(trace_id.shared).to eq(true) + end + end + context 'trace_id_128bit is false' do let(:traceid) { '5af30660491a5a27234555b04cf7e099' } @@ -76,6 +84,15 @@ expect(trace_id.trace_id.to_s).to eq(traceid) end end + + describe '#to_s' do + it 'returns all information' do + expect(trace_id.to_s).to eq( + 'TraceId(trace_id = 234555b04cf7e099, parent_id = f0e71086411b1445, span_id = c3a555b04cf7e099,' \ + ' sampled = true, flags = 0, shared = false)' + ) + end + end end describe Trace::TraceId128Bit do @@ -118,45 +135,79 @@ describe Trace::Span do let(:span_id) { 'c3a555b04cf7e099' } let(:parent_id) { 'f0e71086411b1445' } - let(:annotations) { [ - Trace::Annotation.new(Trace::Annotation::SERVER_RECV, dummy_endpoint).to_h, - Trace::Annotation.new(Trace::Annotation::SERVER_SEND, dummy_endpoint).to_h - ] } + let(:timestamp) { 1452987900000000 } + let(:duration) { 0 } + let(:key) { 'key' } + let(:value) { 'value' } + let(:numeric_value) { 123 } let(:span_without_parent) do Trace::Span.new('get', Trace::TraceId.new(span_id, nil, span_id, true, Trace::Flags::EMPTY)) end let(:span_with_parent) do Trace::Span.new('get', Trace::TraceId.new(span_id, parent_id, span_id, true, Trace::Flags::EMPTY)) end - let(:timestamp) { 1452987900000000 } - let(:duration) { 0 } - let(:key) { 'key' } - let(:value) { 'value' } - let(:numeric_value) { 123 } - let(:boolean_value) { true } before do Timecop.freeze(Time.utc(2016, 1, 16, 23, 45)) [span_with_parent, span_without_parent].each do |span| - annotations.each { |a| span.annotations << a } + span.kind = Trace::Span::Kind::CLIENT + span.local_endpoint = dummy_endpoint + span.remote_endpoint = dummy_endpoint + span.record(value) + span.record_tag(key, value) end - allow(Trace).to receive(:default_endpoint).and_return(Trace::Endpoint.new('127.0.0.1', '80', 'service_name')) end describe '#to_h' do - it 'returns a hash representation of a span' do - expected_hash = { - name: 'get', - traceId: span_id, - id: span_id, - annotations: annotations, - binaryAnnotations: [], - debug: false, - timestamp: timestamp, - duration: duration - } - expect(span_without_parent.to_h).to eq(expected_hash) - expect(span_with_parent.to_h).to eq(expected_hash.merge(parentId: parent_id)) + context 'client span' do + let(:expected_hash) do + { + name: 'get', + kind: 'CLIENT', + traceId: span_id, + localEndpoint: dummy_endpoint.to_h, + remoteEndpoint: dummy_endpoint.to_h, + id: span_id, + debug: false, + timestamp: timestamp, + duration: duration, + annotations: [{ timestamp: timestamp, value: "value" }], + tags: { "key" => "value" } + } + end + + it 'returns a hash representation of a span' do + expect(span_without_parent.to_h).to eq(expected_hash) + expect(span_with_parent.to_h).to eq(expected_hash.merge(parentId: parent_id)) + end + end + + context 'server span' do + let(:shared_server_span) do + Trace::Span.new('get', Trace::TraceId.new(span_id, nil, span_id, true, Trace::Flags::EMPTY, true)) + end + let(:expected_hash) do + { + name: 'get', + kind: 'SERVER', + traceId: span_id, + localEndpoint: dummy_endpoint.to_h, + id: span_id, + debug: false, + timestamp: timestamp, + duration: duration, + shared: true + } + end + + before do + shared_server_span.kind = Trace::Span::Kind::SERVER + shared_server_span.local_endpoint = dummy_endpoint + end + + it 'returns a hash representation of a span' do + expect(shared_server_span.to_h).to eq(expected_hash) + end end end @@ -177,66 +228,42 @@ end describe '#record_tag' do - it 'records a binary annotation' do + it 'records a tag' do span_with_parent.record_tag(key, value) - ann = span_with_parent.binary_annotations[-1] - expect(ann.key).to eq('key') - expect(ann.value).to eq('value') + tags = span_with_parent.tags + expect(tags[key]).to eq('value') end - it 'converts the value to string' do + it 'allows a numeric value' do span_with_parent.record_tag(key, numeric_value) - ann = span_with_parent.binary_annotations[-1] - expect(ann.value).to eq('123') - end - - it 'does not convert the boolean value to string' do - span_with_parent.record_tag(key, boolean_value, Trace::BinaryAnnotation::Type::BOOL) - - ann = span_with_parent.binary_annotations[-1] - expect(ann.value).to eq(true) + tags = span_with_parent.tags + expect(tags[key]).to eq(123) end end describe '#record_local_component' do - it 'records a binary annotation ' do + it 'records a local_component tag' do span_with_parent.record_local_component(value) - ann = span_with_parent.binary_annotations[-1] - expect(ann.key).to eq('lc') - expect(ann.value).to eq('value') + tags = span_with_parent.tags + expect(tags['lc']).to eq('value') end end end describe Trace::Annotation do - let(:annotation) { Trace::Annotation.new(Trace::Annotation::SERVER_RECV, dummy_endpoint) } + let(:annotation) { Trace::Annotation.new(Trace::Span::Tag::ERROR) } describe '#to_h' do before { Timecop.freeze(Time.utc(2016, 1, 16, 23, 45)) } it 'returns a hash representation of an annotation' do expect(annotation.to_h).to eq( - value: 'sr', - timestamp: 1452987900000000, - endpoint: dummy_endpoint.to_h - ) - end - end - end - - describe Trace::BinaryAnnotation do - let(:annotation) { Trace::BinaryAnnotation.new('http.path', '/', 'STRING', dummy_endpoint) } - - describe '#to_h' do - it 'returns a hash representation of a binary annotation' do - expect(annotation.to_h).to eq( - key: 'http.path', - value: '/', - endpoint: dummy_endpoint.to_h + value: 'error', + timestamp: 1452987900000000 ) end end diff --git a/spec/lib/zipkin_tracer_base_spec.rb b/spec/lib/zipkin_tracer_base_spec.rb index 1ad9836..6a2b07c 100644 --- a/spec/lib/zipkin_tracer_base_spec.rb +++ b/spec/lib/zipkin_tracer_base_spec.rb @@ -10,17 +10,20 @@ let(:rpc_name) { 'this_is_an_rpc' } let(:previous_rpc_name) { 'this_is_previous_rpc' } let(:tracer) { described_class.new } + let(:default_endpoint) { Trace::Endpoint.new('127.0.0.1', '80', 'service_name') } let(:span_hash) { { name: rpc_name, traceId: span_id, id: span_id, - annotations: [], - binaryAnnotations: [], + localEndpoint: default_endpoint.to_h, timestamp: 1452987900000000, duration: 0, debug: false } } - before { Timecop.freeze(Time.utc(2016, 1, 16, 23, 45, 0)) } + before do + Timecop.freeze(Time.utc(2016, 1, 16, 23, 45, 0)) + allow(::Trace).to receive(:default_endpoint).and_return(default_endpoint) + end describe '#flush!' do it 'raises if not implemented' do @@ -34,18 +37,21 @@ it 'sets the span name' do expect(span.name).to eq(rpc_name) end + it 'sets the local endpoint' do + expect(span.local_endpoint).to eq(default_endpoint) + end context "no parentId" do it 'returns an empty span' do - expect(span.binary_annotations).to eq([]) expect(span.annotations).to eq([]) + expect(span.tags).to eq({}) expect(span.to_h).to eq(span_hash) end end context "with parentId" do let(:span) { tracer.start_span(trace_id_with_parent, rpc_name) } it 'returns an empty span' do - expect(span.binary_annotations).to eq([]) expect(span.annotations).to eq([]) + expect(span.tags).to eq({}) expect(span.to_h).to eq(span_hash.merge({parentId: parent_id})) end end @@ -57,7 +63,6 @@ describe '#end_span' do let(:span) { tracer.start_span(trace_id, rpc_name) } - before { allow(Trace).to receive(:default_endpoint).and_return(Trace::Endpoint.new('127.0.0.1', '80', 'service_name')) } it 'closes the span' do span #touch it so it happens before we freeze time again Timecop.freeze(Time.utc(2016, 1, 16, 23, 45, 1)) @@ -65,20 +70,14 @@ tracer.end_span(span) expect(span.to_h).to eq(span_hash.merge(duration: 1_000_000)) end - it 'flush if SS is annotated in this span' do - span.record(Trace::Annotation::SERVER_SEND) - expect(tracer).to receive(:flush!) - expect(tracer).to receive(:reset) - tracer.end_span(span) - end - it 'flush if CR is annotated in this span and the span does not have parent span' do - span.record(Trace::Annotation::CLIENT_RECV) + it 'flush if kind is SERVER in this span' do + span.kind = Trace::Span::Kind::SERVER expect(tracer).to receive(:flush!) expect(tracer).to receive(:reset) tracer.end_span(span) end - it "flush if SS has not been annotated but span has no parent span " do - span.record(Trace::Annotation::SERVER_RECV) + it 'flush if kind is CLIENT in this span and the span does not have parent span' do + span.kind = Trace::Span::Kind::CLIENT expect(tracer).to receive(:flush!) expect(tracer).to receive(:reset) tracer.end_span(span) @@ -87,9 +86,8 @@ describe '#end_span with parent span' do let(:span) { tracer.start_span(trace_id_with_parent, rpc_name) } - before { allow(Trace).to receive(:default_endpoint).and_return(Trace::Endpoint.new('127.0.0.1', '80', 'service_name')) } it 'does not flush if the current span has a parent span' do - span.record(Trace::Annotation::CLIENT_RECV) + span.kind = Trace::Span::Kind::CLIENT expect(tracer).not_to receive(:flush!) expect(tracer).not_to receive(:reset) tracer.end_span(span)