Context api dummy methods #155

satac2 · 2020-07-06T19:45:58Z

Context api with just dummy methods, runtime and threadlocal context wrappers to come next as well as the context sdk implementation, which will contain the actual methods.

codecov · 2020-07-07T20:10:26Z

Codecov Report

Merging #155 into master will increase coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #155      +/-   ##
==========================================
+ Coverage   94.17%   94.27%   +0.09%     
==========================================
  Files          76       76              
  Lines        1820     1851      +31     
==========================================
+ Hits         1714     1745      +31     
  Misses        106      106

Impacted Files	Coverage Δ
sdk/test/trace/tracer_test.cc	`97.22% <0.00%> (-0.43%)`	⬇️
sdk/include/opentelemetry/sdk/trace/tracer.h	`100.00% <0.00%> (ø)`
api/include/opentelemetry/trace/noop.h	`61.53% <0.00%> (+3.84%)`	⬆️
sdk/src/trace/tracer.cc	`72.00% <0.00%> (+13.17%)`	⬆️

api/include/opentelemetry/context/context.h

reyang

LGTM. Once we have the TLS (thread local) one, we might want to make Context an abstract class to prevent accidental instantiation.

maxgolov · 2020-07-07T21:01:14Z

api/include/opentelemetry/context/context.h

+  // The Key class is used to obscure access from the
+  //  user to the context map. The identifier is used as a key
+  // to the context map.
+  class Key


I'm a bit lost as to what is the real purpose of the Key class. Why not using nostd::string_view instead? There's a recent commit that is gonna add ability to use nostd::string_view in both std::map and std::unordered_map . Do I always have to use the Context::Key if I need to populate some value in the context? This looks a bit unnatural to me.

For example, OT Java SDK - decided that they will convert keys to simple string, very recent commit: open-telemetry/opentelemetry-java@24b4be9 - from a practical standpoint it is much easier to deal with "standard" (or close to standard) primitive than creating a custom Key class.

Why not using nostd::string_view instead?

Sam's not opinionated on the issue, so we'll need a concrete recommendation from a maintainer.

I suggested a Key object because the spec says "The API MUST return an opaque object representing the newly created key.", and be setup such that "Multiple calls to CreateKey with the same name SHOULD NOT return the same value". If we use a string as the key, clients are going to simply use the key name as the key itself, violating that requirement.

@0x4b - This is bad for performance. See: open-telemetry/opentelemetry-java#1314

I'd suggest we follow more closely Java implementation than Python or Go, as C++ structurally closer to Java. The issue of Context keys brought up here for Java: open-telemetry/opentelemetry-java#1334 , and spec authors signed off on it allowing String as key for CorrelationContext. I think it'd be the right solution for C++ as well, at least from performance standpoint.

What is your opinion on those two commits above?

If this this is bad for performance in Java, it doesn't extrapolate to C++. In C++ a struct containing a string has no storage or copy overhead compared to the string itself. If there's a concern that string is too expensive, the key can be built around a string_view.

@0x4b - we can probably use type alias for the Key class to nostd::string_view. Any complex application, even with thousands events has a limited number of known beforehand context keys and event attributes. Initializing these from a string literal as nostd::string_view does not require a copy, as storage of these keys does not go out of scope. Recordable or Exporter dealing with async processing of incoming spans / events can perform a memcopy as-needed.

@reyang I am removing the Key Object as we spoke about, but I wanted to make sure I did not misinterpret our conversation on Friday and that using a string as a key is the way we want to go.

maxgolov · 2020-07-07T21:02:57Z

api/include/opentelemetry/context/context.h

+  }
+
+  // Returns the value associated with the passed in key.
+  virtual common::AttributeValue GetValue(Key key) { return 0; }


It would be more elegant and in C++ spirit to use [] operator and nostd::string_view to obtain the key, as it's done in a map. Since we're dealing with dictionary, it'd be more practical this dictionary to have std::map-alike properties, like access operators.

Map operator[] returns a T& (non-const) and constructs an element dynamically if it's not already present.

@maxgolov Are those behavior you want for Context?

@0x4b - but this empty default implementation also returns 0 for non-existing keys, and doesn't describe expected behavior in case if attribute does not exist in CorrelationContext, for example. We could say that if the key does not exist - we either throw (std::out_of_range-alike exception) or return a NULL object. We can allow a special AttributeValue of null, in that case the operator can return a reference to constructed NULL object.

I didn't mean my question to be rhetorical. I don't know what behavior OT wants for value lookup.

In another PR you asked for support for environments that don't support exceptions, so it seems like there needs to be an option other than throwing here. The options I see are

optional<>, but I'm assuming you're not interested in a nostd::optional. Is that right?

a valueless [no]std::variant (assuming that's the value type). that feels kind of unnatural

a default-constructed AttributeValue (i.e. false bool variant)

a pointer to the value type

Whether to use string_view for the key is it's own question, and there's also an open question about what the value type should be, so maybe that discussion will resolve in a way that implies what an unknown value should be.

I think we can discuss:

valueless nostd::variant as NULL or Empty object as a preference for both exceptions and no-exceptions build - provides consistent behavior for both flavors; OR

throw on exceptions build, and THROW macro as a macro == std::terminate on no-exceptions build - less preferred, since API behavior would be different.

I shared some code below providing an implementation with both suggestions, including a placeholder for NULL-object in a variant. Default constructed Value should be NULL-or-empty object, I think it's gonna be fairly natural. It's empty until you assign something to it.

pyohannes · 2020-07-07T21:15:44Z

api/include/opentelemetry/context/context.h

+  }
+
+  // Returns the value associated with the passed in key.
+  virtual common::AttributeValue GetValue(Key key) { return 0; }


I'm doubtful whether common::AttributeValue will cover all use cases. One use case might be storing the current Span or SpanContext, and this is not covered by the AttributeValue variant.

What might we return instead of common::AttributeValue?

opentelemetry/nostd/variant.h

I'm not completely sure.

One option might be to define a list of all value types we need to support and then add another context specific variant.

Would it be valuable to have a struct for SpanContext and just allow it be in common::AttributeValue ? Or do you see value in having generic any key-value context be nested in a variant? I mean CorrelationContext is probably just a dictionary of existing nostd::variant as long as we add SpanContext to it?

One use case might be storing the current Span or SpanContext

I pointed Sam at AttributeValue (on gitter) based on the assumption that spans would be by ID (not as an object) using a specific key in the current context, and when the span changes the new ID is stored for that key.

If it's not clear what the set of value types needs to be, can we start with
using ContextValue = AttributeValue;
until the issue is resolved (either be removing the type alias or defining ContextValue concretely?

Can we make different assumptions about the context types:

Context

SpanContext

being really just structs, possibly with getters/setters.

And only:

CorrelationContext

being a dictionary, in traditional C++ sense of it? (map or unordered_map, with extra validation of input constraints on key / naming convention of it, possibly throwing or terminating for invalid key names)

I looked at how Java OT operates with Jaeger exporter, and how JaegerSpanContext is implemented: pretty much a "struct" (Plain Java Object), with helper getter / setters for input / output validation. Can we consider that some context types are more "struct"-alike, and the other is more "dictionary"-alike? We can spec common getter / setter pattern for these, if needed make them share common ancestor, but semantically the best type for a dictionary in C++ is likely std::unordered_map<nostd::string_view, common::AtributeValue> ... Would there be a concern if span event attributes and correlation context attributes be of the same common variant type? ... Re: std::unordered_map - we do not have to make it ABI-safe, if we ensure that the SDK methods that operate on contexts treat them as KeyValueIterableView type. For w3c Trace Context - it appears like we either need a string, and for Context - if we are missing byte array, we should add nostd::span<uint8_t> to AttributeValue to allow passing byte arrays. Even if spec doesn't allow byte arrays on wire, we might have a few use-cases where having byte array (specifically being byte with possible 0-byte) instead of existing string type.

Who needs to be involved to make the decision on this?

Let's discuss this topic on Monday community meeting.

satac2 · 2020-07-09T16:05:50Z

I am looking to move forwards on this, can we get resolution on whether we would like to keep the Key class or simply use a nostd::string_view?

Co-authored-by: Reiley Yang <reyang@microsoft.com>

maxgolov

I'd suggest we go with what Java OT SDK did:

they got blessing from spec authors to use regular String type for CorrelationContext dictionary
it is bad for perf to add unnecessary wrappers
consider Context and SpanContext having just 2- or 4- beforehand known props be just structs, with common accessors (getters / setters) from base Context interface
consider CorrelationContext being a dictionary with access operators [] and allowing to use nostd::string_view created from string literals for keys. This will be good for perf. Validation rules can be added later on to throw in the operator in case if key name doesn't meet the input/validation constraints.

maxgolov · 2020-07-10T10:22:01Z

This is what I have in mind - draft, not final, works. std types where possible should be nostd. But not necessarily - app may safely use 'native' standard library types (std::map) everywhere, assuming that there's a template method that performs transform from app view of the Context to ABI-stable type (such as KeyValueIterable) on API call into SDK. Basically SDK should accept contexts either as Value objects (e.g. C structs with specified alignment / packing) or as KeyValueIterable. Concrete API call in SDK knows what kind of context it is, thus should unwrap correspondingly.

It would help to see some concrete examples how (when, how often, what use-cases) - context info for both different context types gets propagated from app to prebuilt SDK.

Concurrency issues should also be addressed.

#include <cstdint>
#include <functional>
#include <iomanip>
#include <iostream>
#include <string>
#include <string_view>
#include <type_traits>
#include <unordered_map>
#include <variant>

static constexpr uint32_t hashCode(const char *str, uint32_t h = 0)
{
  return (uint32_t)(!str[h] ? 5381 : ((uint32_t)hashCode(str, h + 1) * (uint32_t)33) ^ str[h]);
}

#define CONST_UINT32_T(x) integral_constant<uint32_t, (uint32_t)x>::value
#define CONST_HASHCODE(name) CONST_UINT32_T(hashCode(#name))

#define TYPE_EQ(x) template <typename = typename std::enable_if<std::is_same<T, x>::value>::type>
#define TYPE_NEQ(x) template <typename = typename std::enable_if<!std::is_same<T, x>::value>::type>

using namespace std;

// could be nostring_view
using Key     = string;
using TraceId = string;
using SpanId  = string;

class NullValue
{};

// TraceId, SpanId could be added here, but for now both are treated as string
using Value = variant<string, uint8_t, uint32_t, uint64_t, NullValue>;

namespace std
{
string to_string(Value v)
{
  switch (v.index())
  {
    case 0:
      return get<string>(v);
    case 1:
      return to_string(get<uint8_t>(v));
    case 2:
      return to_string(get<uint32_t>(v));
    case 3:
      return to_string(get<uint64_t>(v));
    default:
      break;
  }
  return "";
}
}  // namespace std

using Map = unordered_map<Key, Value>;

using Callback = function<bool(Key, Value)>;

// Read-only iterable
class KeyValueIterable
{
public:
  virtual ~KeyValueIterable() = default;

  /**
   * Iterate over key-value pairs
   * @param callback a callback to invoke for each key-value. If the callback returns false,
   * the iteration is aborted.
   * @return true if every key-value pair was iterated over
   */
  virtual bool ForEachKeyValue(Callback callback) const noexcept = 0;

  /**
   * @return the number of key-value pairs
   */
  virtual size_t size() const noexcept = 0;
};

// Dictionary facet
class Dictionary : public KeyValueIterable
{
public:
  virtual Value GetProperty(Key key)             = 0;
  virtual void SetProperty(Key key, Value value) = 0;
  virtual void RemoveProperty(Key key)           = 0;
};

// Empty dictionary implementation
class NullDictionary : public Dictionary
{
public:
  virtual Value GetProperty(Key key) override { return {NullValue()}; };
  virtual void SetProperty(Key key, Value value) override{};
  virtual void RemoveProperty(Key key) override{};
  virtual bool ForEachKeyValue(Callback callback) const noexcept override { return true; };
  virtual size_t size() const noexcept override { return 0; };
};

// SpanContext is a C struct. This may even live in a C header rather than C++.
// ABI-safety considerations:
// - Safe to be passed across ABI assuming that TraceId and SpanId storage types are ABI-safe, OR
// - Must be transformed into KeyValueIterable on a call from App to prebuilt SDK implementation.
//
typedef struct SpanContext
{
  TraceId traceID;
  SpanId spanID;
  uint64_t traceFlags;
  uint64_t traceState;
} SpanContext;

//
// CorrelationContext is a map. App owns and app threads modify that map.
// Default implementation is NOT thread-safe.
// Windows users may substitute this for concurrent_unordered_map.
//
// ABI-safety considerations:
// - Must be transformed into KeyValueIterable on a call from App to prebuilt SDK implementation.
using CorrelationContext = Map;

// Wrapper object to enable dictionary access to SpanContext
class SpanContextDictionary : public Dictionary
{
  SpanContext &context;

public:
  SpanContextDictionary(SpanContext &src) : context(src){};

  /**
   *
   */
  virtual Value GetProperty(Key key) override
  {
    switch (hashCode(key.data()))
    {
      case CONST_HASHCODE("traceID"):
        return {context.traceID};
      case CONST_HASHCODE("spanID"):
        return {context.spanID};
      case CONST_HASHCODE("traceFlags"):
        return {context.traceFlags};
      case CONST_HASHCODE("traceState"):
        return {context.traceState};
      default:
        break;
    }
    return {NullValue()};
  }

  /**
   *
   */
  virtual void SetProperty(Key key, Value value) override
  {
    switch (hashCode(key.data()))
    {
      case CONST_HASHCODE("traceID"):
        context.traceID = std::get<0>(value);
        break;
      case CONST_HASHCODE("spanID"):
        context.spanID = std::get<0>(value);
        break;
      case CONST_HASHCODE("traceFlags"):
        context.traceFlags = std::get<3>(value);
        break;
      case CONST_HASHCODE("traceState"):
        context.traceState = std::get<3>(value);
        break;
      default:
        // throw???
        break;
    }
  }

  /**
   *
   */
  virtual bool ForEachKeyValue(Callback callback) const noexcept override
  {
    bool result = true;
    result &= callback("traceID", {context.traceID});
    result &= callback("spanID", {context.spanID});
    result &= callback("traceFlags", {context.traceFlags});
    result &= callback("traceState", {context.traceState});
    return result;
  }

  virtual size_t size() const noexcept override { return 4; }

  virtual void RemoveProperty(Key key) override
  {
    // TODO: throw?
  }
};

//
// Wrapper object to enable dictionary access to CorrelationContext
// TODO: consider making Dictionary implementation to be always thread-safe?
// Someone who does not need thread-safety - may operate on CorrelationContext.
//
class CorrelationContextDictionary : public Dictionary
{
  CorrelationContext &context;

public:
  CorrelationContextDictionary(CorrelationContext &src) : context(src){};

  /**
   *
   */
  virtual Value GetProperty(Key key) override
  {
    // lockguard?
    return context[key];
  }

  /**
   *
   */
  virtual void SetProperty(Key key, Value value) override
  {
    // lockguard?
    context[key] = value;
  }

  /**
   *
   */
  virtual void RemoveProperty(Key key) override
  {
    // lockguard?
    // TODO: map.erase
  }

  /**
   *
   */
  virtual bool ForEachKeyValue(Callback callback) const noexcept override
  {
    // lockguard?
    bool result = true;
    for (const auto &kv : context)
    {
      result &= callback(kv.first, kv.second);
    }
    return result;
  }

  virtual size_t size() const noexcept override
  {
    // lockguard?
    return context.size();
  }
};

/**
 * Demo!
 */
#define LEFT10 left << setw(10)

int main(int argc, char **argv)
{
  cout << "Hello, Context!" << endl;

  {
    cout << "- SpanContext:" << endl;

    SpanContext ctx;
    ctx.traceID    = "foo";
    ctx.spanID     = "bar";
    ctx.traceFlags = 0x1234;
    ctx.traceState = 0;

    cout << "-- As a struct:" << endl;
    cout << LEFT10 << "traceID"
         << "=" << ctx.traceID << endl;
    cout << LEFT10 << "spanID"
         << "=" << ctx.spanID << endl;
    cout << LEFT10 << "traceFlags"
         << "=" << ctx.traceFlags << endl;
    cout << LEFT10 << "traceStat"
         << "=" << ctx.traceState << endl;

    cout << "-- As a dictionary/KeyValueIterable:" << endl;
    cout << "SpanContextDictionary:" << endl;
    SpanContextDictionary ctxD(ctx);
    ctxD.ForEachKeyValue([=](Key key, Value value) -> bool {
      cout << LEFT10 << key << "=" << to_string(value) << endl;
      return true;
    });
  };

  {
    cout << "- CorrelationContext:" << endl;

    CorrelationContext ctx;
    ctx["foo"] = "someFoo";
    ctx["bar"] = "someBar";
    ctx["baz"] = (uint8_t){123};

    cout << "-- As a map:" << endl;
    for (auto &kv : ctx)
    {
      cout << LEFT10 << kv.first << "=" << to_string(kv.second) << endl;
    }

    cout << "-- As a dictionary/KeyValueIterable:" << endl;
    CorrelationContextDictionary ctxD(ctx);
    ctxD.ForEachKeyValue([=](Key key, Value value) -> bool {
      cout << LEFT10 << key << "=" << to_string(value) << endl;
      return true;
    });
  }
  return 0;
}

maxgolov · 2020-07-10T18:12:37Z

@0x4b @satac2 - we could use CreateKey API to add ownership semantics to something that would've been otherwise a non-owning nostd::string_view. Can we assume that the Context APIs allow either nostd::string_view or std::string (which is totally fine if there's a transform of keys for-each via KeyValueIterable - for passing object across ABI as nostd::string_view key on top of that string buffer across ABI boundary).

BUT - it is also possible to use CreateKey to actually create a key dynamically (rather than string_view created from const string buffer from string literal). That way we know that Key object is owning flavor of string_view : when it goes out of scope, the string buffer is freed.

With this approach we can either use Key as an argument to Context fields / properties setter API (which must be a subclass of string_view), OR plain string_view interchangeably. Then CreateKey can have a property of performing regexp / constraints check on a key, whereas regular string_view will not perform key input checking constraints.. Then depending on whether it is a requirement or not, we can perform key checking constraints in the assignment / operator.

I think of Context / Dictionary-style API being somewhat an optional "safety" syntactic sugar, since fundamentally SpanContext - can be easily and more naturally expressed as a plain struct. And CorrelationContext - is a (possibly and most likely thread-safe?) map. Those needing syntactic sugar can add it, and those needing performance - can skip adding it.

While passing Context objects from app to SDK --- SDK then accepts a transform (done via template) of original map object to KeyValueIterable, as this is the only ABI-safe container, similar to what we do on Span::AddEvent API.

…emetry-cpp into context_api_dummy_methods

satac2 · 2020-07-13T16:26:16Z

@reyang @maxgolov @pyohannes I have updated the PR to remove the Key Object, and instead just accept a string as the key. I have also added a ContextValue variant, which currently is identical to AttributeValue but provides the capability to add whatever is desired as needs arise. Also, I added the context implementation to the api so they are no longer dummy methods.

reyang · 2020-07-13T16:52:05Z

api/include/opentelemetry/context/context.h

+  Context SetValue(nostd::string_view key, T &value) noexcept
+  {
+    std::map<std::string, context::ContextValue> context_map_copy = context_map_;
+    context_map_copy[std::string(key)]                            = value;


What would happen if a library (compilation unit A) is making a call to this method, while the context object was constructed by another binary (compilation unit B), while A and B are using different version of STL?

I think I solved the issue by putting the map into a KeyValueIterableView, and then copying the data over.

reyang

The implementation seems to be problematic from ABI compat perspective, please find my comments.

maxgolov

I think we need to take another stab at adding std::initializer_list<std::pair<nostd::string_view, context::ContextValue>> as optional parameter to non-default constructor of Context - that'd be similar to what was recently added to Span API.

But conceptually it looks good. We may consider adding:

operators
optional no-value (NULL-value)
provision for (optional) key input constraints checking. Although this may be done in SDK, not API).

maxgolov

I'd like to reset my feedback for now:

can we consider adding a using Map = std::map<std::string, context::ContextValue>> to replace this with alternate (possibly thread-safe?) map implementation later on? Is there a requirement in OT for Context to be possibly concurrently accessed by different threads?
can we avoid context_map_copy until we actually need to pass the object to SDK? There we'd need a templated converter from local app api::Context to sdk::Context via Key-Value Iterable templated method in SDK header, in concrete API call where Context is going to be used as input argument.

…emetry-cpp into context_api_dummy_methods

satac2 · 2020-07-14T15:29:22Z

@reyang all checks are passing and I believe I have fixed the ABI compatibility issues. Could you take another look?

satac2 added 3 commits July 6, 2020 19:35

added context header with dummy methods

4128a69

Build and CMake files

cba6d5f

auto formatted

b2e876e

satac2 requested a review from a team July 6, 2020 19:45

satac2 closed this Jul 6, 2020

satac2 reopened this Jul 7, 2020

reyang reviewed Jul 7, 2020

View reviewed changes

api/include/opentelemetry/context/context.h Outdated Show resolved Hide resolved

reyang reviewed Jul 7, 2020

View reviewed changes

api/include/opentelemetry/context/context.h Outdated Show resolved Hide resolved

reyang approved these changes Jul 7, 2020

View reviewed changes

maxgolov reviewed Jul 7, 2020

View reviewed changes

pyohannes reviewed Jul 7, 2020

View reviewed changes

satac2 and others added 2 commits July 9, 2020 20:03

changed variable name from attributes to values

a7480c8

Update api/include/opentelemetry/context/context.h

96cd32f

Co-authored-by: Reiley Yang <reyang@microsoft.com>

maxgolov self-requested a review July 9, 2020 21:34

maxgolov suggested changes Jul 9, 2020

View reviewed changes

satac2 added 6 commits July 13, 2020 04:02

changed to follow the spec and added context_value type

5f0634b

Merge branch 'context_api_dummy_methods' of github.com:satac2/opentel…

5d77daf

…emetry-cpp into context_api_dummy_methods

removed tests for this PR

68a1a70

removed line

5e68574

removed unnecessary include

16a492e

changed a function to pass by reference

497eb67

satac2 added 2 commits July 13, 2020 16:32

formatted

479dead

Added tests.

2a78c48

reyang reviewed Jul 13, 2020

View reviewed changes

reyang requested changes Jul 13, 2020

View reviewed changes

avoiding ABI compatibility issues

f51748f

maxgolov approved these changes Jul 13, 2020

View reviewed changes

Merge branch 'master' into context_api_dummy_methods

7d73219

maxgolov reviewed Jul 13, 2020

View reviewed changes

satac2 added 3 commits July 14, 2020 14:50

added throw capture

3f85aa5

Merge branch 'context_api_dummy_methods' of github.com:satac2/opentel…

0ec83e8

…emetry-cpp into context_api_dummy_methods

minor exception syntax error

21ea872

reyang self-requested a review July 20, 2020 22:40

reyang added pr:please-merge This PR is ready to be merged by a Maintainer (rebased, CI passed, has enough valid approvals, etc.) and removed pr:please-merge This PR is ready to be merged by a Maintainer (rebased, CI passed, has enough valid approvals, etc.) labels Jul 20, 2020

satac2 closed this Jul 28, 2020

Context api dummy methods #155

Context api dummy methods #155

Conversation

satac2 commented Jul 6, 2020

codecov bot commented Jul 7, 2020 • edited Loading

Codecov Report

reyang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxgolov Jul 9, 2020 • edited Loading

Choose a reason for hiding this comment

0x4b Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

maxgolov Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxgolov Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

satac2 commented Jul 9, 2020

maxgolov left a comment

Choose a reason for hiding this comment

maxgolov commented Jul 10, 2020 • edited Loading

maxgolov commented Jul 10, 2020 • edited Loading

satac2 commented Jul 13, 2020

reyang Jul 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyang left a comment

Choose a reason for hiding this comment

maxgolov left a comment

Choose a reason for hiding this comment

maxgolov left a comment • edited Loading

Choose a reason for hiding this comment

satac2 commented Jul 14, 2020

codecov bot commented Jul 7, 2020 •

edited

Loading

maxgolov Jul 9, 2020 •

edited

Loading

0x4b Jul 10, 2020 •

edited

Loading

maxgolov Jul 10, 2020 •

edited

Loading

maxgolov Jul 10, 2020 •

edited

Loading

maxgolov commented Jul 10, 2020 •

edited

Loading

maxgolov commented Jul 10, 2020 •

edited

Loading

reyang Jul 13, 2020 •

edited

Loading

maxgolov left a comment •

edited

Loading