Skip to content
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

Proposal: Focus on functionality/capabilities rather than how #165

Closed
bogdandrutu opened this issue Jun 27, 2019 · 11 comments
Closed

Proposal: Focus on functionality/capabilities rather than how #165

bogdandrutu opened this issue Jun 27, 2019 · 11 comments

Comments

@bogdandrutu
Copy link
Member

While discussing with some of the community members (e.g. @jmacd) there were some issues about implementing a functionality in a specific way (e.g. this object MUST be an interface).

Here is an example of how the AddLink can be re-written:


See Links description. Linked Span can be from the same or different trace.

A Link is defined by:

  • (Required) SpanContext of the Span to link to
  • (Optional) Map of attributes associated with this link. Attributes are key:value pairs where hey is a string and value is one of string, boolean and numeric.

The Span interface:

  • Provide an API to record links where the link properties are passed as arguments. This SHOULD be called AddLink.
  • Provide an API to record lazy initialized links. This can be implemented by having a Link interface or by providing a concrete Link definition and a LazyLinkFormatter. If languages supports overloads then this SHOULD be called AddLink otherwise AddLazyLink may be consider.

Similar suggestion can be made for SpanBuilder. I think we should not enforce that every language uses a Builder pattern. We should rephrase the Tracer capability to be: "Allow span creation with the following options configured: required a name, allow to set an explicit parent, etc.". So language can for example have an API Span Tracer::StartSpan(SpanStartOptions);

Disclaimer: What I suggested may not be the right way to re-write specifications but I tried to show a possible way to do this.

I would like to know if TC members and others agree with:

  1. Every language should use the language specific patterns to achieve the functionality define in the specification.

Why?

  1. I think this way every language API and SDK can be more language idiomatic and allow languages like golang to not pay unnecessary performance overhead when working with interfaces (extra heap allocations).
@bogdandrutu bogdandrutu changed the title Proposal: Focus on capabilities rather then how Proposal: Focus on functionality/capabilities rather than how Jun 27, 2019
@SergeyKanzhelev
Copy link
Member

Definitely it will make specs better. Good feedback.

@jmacd
Copy link
Contributor

jmacd commented Jun 28, 2019

Agreed! Here's another example:

I've been thinking about the current SpanData issues, and realized that I've seen more than one requirement to justify the current design. The apparent requirements are:

(1) Provide a way to specify out-of-band or asynchronous spans, including control over their resources and timestamps, and potentially a way to indicate non-local behavior (e.g., to avoid attaching local stacktraces).

(2) Provide a way to record lazy spans such that the sampler is given an opportunity to reject them before spending the CPU/memory to compute them.

I wasn't aware of this second requirement until recently, and it's changed my thinking about SpanData somewhat. If there is indeed a requirement to support lazy spans, then we should be very clear about how the issue is inseparable from the sampling API (IIUC).

@bogdandrutu
Copy link
Member Author

@jmacd let's not mix the issues, please copy in the SpanData issue comments related to that to make sure we have all thoughts about that in one place.

Here I am trying to just make sure everyone is ok with changing the way we write the Specs and what we require from every language "implement the same functionality/capability across all the languages in the most language idiomatic way"

@jmacd
Copy link
Contributor

jmacd commented Jun 28, 2019

It was an example. We will put the SpanData argument into a RFC.

@bogdandrutu
Copy link
Member Author

If you are ok with the proposal please +1/Like the initial description so that we can achieve some kind of agreement and move forward with some PRs and issues to implement this.

@tigrannajaryan
Copy link
Member

Focus on functionality/capabilities rather than how

@bogdandrutu I completely agree. The specification should aim to be language independent. The final language-specific API should be left open for language library designers to come up with the best patterns which are idiomatic for the each language.

I also find it useful that after the general specification is written, it is great to have examples/suggestions that are specific for some common languages (e.g. Java, Go). Also possibly include very simple reference implementation if needed to illustrate the idea described in the specification.

@bogdandrutu
Copy link
Member Author

@tigrannajaryan that is a very good suggestion to link to some implementations. Unfortunately for the moment only Java has them (Node is getting close but probably I would use go as the second example when that is done).

@c24t
Copy link
Member

c24t commented Jun 28, 2019

Describing the classes in terms of the intended behavior instead of prescribing actual methods or patterns (as in java's SpanBuilder) sounds exactly right to me.

It's not surprising that the specs would reflect the java implementation now since it's the only client with a complete API package, as we write the other language clients we can apply the lessons we learn to the spec.

@rochdev
Copy link
Member

rochdev commented Jul 9, 2019

I definitely agree with a more behavior oriented and more flexible specification.

In the case of the JavaScript implementation, so far most reviews have looked something like this:

  • Concern: this API doesn't make sense for JavaScript, and may even make it impossible to implement by APM vendors
  • Answer: the spec says that the MUST be implemented like this, so that is how it will be implemented

This is currently leading to an API that is not user nor vendor friendly for JavaScript, and that uses patterns that are considered anti-patterns in the language.

Having a more flexible API that is more focused on how concepts should be named and how they should behave in a more general way should fix this issue in my opinion, and will open discussions by language for better APIs.

I also think the APIs should try to be closer to OpenTracing since it's an established standard and changing terminology will be confusing for a lot of users, but I will open a separate issue for that.

Since there is a lot of progress on the JavaScript front, would it make sense to open an issue in opentelemetry-js with a proposal for an API that would make the most sense for JS, and then revisit the specification based on the outcome? This would give a fresh perspective on the API from a language that is basically the polar opposite of Java.

@jmacd
Copy link
Contributor

jmacd commented Aug 20, 2019

After several spec discussions in recent weeks, I've come to think of this question about how we specify functionality/capabilities as only part of the challenge. When we adopt the OpenTracing requirement to separate the API from the implementation, we are faced with the need for a semantic description of what the API achieves independent of the features and capabilities.

It means we should focus on semantic description at the API level, capabilities description at the SDK level. As an example of how this observation could be helpful, consider there is some disagreement over the term "GetOrCreateTimeSeries" to describe a metrics handle with pre-defined labels. The descriptive phrase "get or create time series" explains the feature or capability that an SDK might use, in this case to "create" something, not the semantics of the operation, which are to get a logical reference to a stream of metrics.

I think focusing on semantics for API specification will help resolve several debates, so I mention it here.

@bogdandrutu
Copy link
Member Author

Closing this issue, we relaxed the way we write Specs.

TuckTuckFloof pushed a commit to TuckTuckFloof/opentelemetry-specification that referenced this issue Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants