Skip to content
This repository has been archived by the owner on Dec 6, 2024. It is now read-only.

Commit

Permalink
Enforce lint, link checks, fix errors. Remove status.
Browse files Browse the repository at this point in the history
Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu committed May 1, 2020
1 parent f9880dd commit eea5f16
Show file tree
Hide file tree
Showing 23 changed files with 578 additions and 482 deletions.
33 changes: 29 additions & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
@@ -1,11 +1,36 @@
version: 2

jobs:
build:
misspell:
docker:
- image: circleci/golang:1.12
- image: circleci/golang:1.14
steps:
- checkout
- run:
name: Verify
command: make precommit
name: Misspell Install
command: make install-misspell
- run:
name: Misspell check
command: make misspell

markdownlint:
docker:
- image: node:13
steps:
- checkout
- run:
name: Install Tools
command: |
make install-markdown-lint
make install-markdown-link-check
- run:
name: Run Tools
command: |
make markdown-lint
make enforce-markdown-link-check
workflows:
version: 2
check-errors:
jobs:
- misspell
- markdownlint
9 changes: 9 additions & 0 deletions .markdownlint.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"default": true,
"MD024": { "allow_different_nesting": true },
"MD029": { "style": "ordered" },
"ul-style": false, # MD004
"line-length": false, # MD013
"no-inline-html": false, # MD033
"fenced-code-language": false # MD040
}
33 changes: 29 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ ALL_DOCS := $(shell find . -name '*.md' -type f | sort)

TOOLS_DIR := ./.tools
MISSPELL_BINARY=$(TOOLS_DIR)/misspell

.PHONY: precommit
precommit: install-misspell misspell
MARKDOWN_LINK_CHECK=markdown-link-check

.PHONY: install-misspell
install-misspell: go.mod go.sum internal/tools.go
install-misspell:
go build -o $(MISSPELL_BINARY) github.com/client9/misspell/cmd/misspell

.PHONY: misspell
Expand All @@ -19,3 +17,30 @@ misspell:
misspell-correction:
$(MISSPELL_BINARY) -w $(ALL_DOCS)

.PHONY: install-markdown-link-check
install-markdown-link-check:
npm install -g $(MARKDOWN_LINK_CHECK)

.PHONY: markdown-link-check
markdown-link-check:
find . -name \*.md -exec sleep 5 \; -exec $(MARKDOWN_LINK_CHECK) {} \;

.PHONY: enforce-markdown-link-check
enforce-markdown-link-check:
@LINKCHECKOUT=`find . -name \*.md -exec sleep 5 \; -exec $(MARKDOWN_LINK_CHECK) {} 2>&1 >/dev/null \;`; \
if [ "$$LINKCHECKOUT" ]; then \
echo "$(MARKDOWN_LINK_CHECK) FAILED => errors:\n"; \
echo "Run 'make $(MARKDOWN_LINK_CHECK)' to see the errors"; \
exit 1; \
else \
echo "Check markdown links finished successfully"; \
fi

.PHONY: install-markdown-lint
install-markdown-lint:
npm install -g markdownlint-cli

.PHONY: markdown-lint
markdown-lint:
markdownlint -c .markdownlint.yaml '**/*.md'

2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ Have suggestions? Concerns? Questions? **Please** raise an issue or raise the ma

## Background on the OpenTelemetry OTEP process

Our OTEP process borrows from the [Rust RFC](https://github.com/rust-lang/rfcs) and [Kubernetes Enhancement Proposal](https://github.com/kubernetes/enhancements) processes, the former also being [very influential](https://github.com/kubernetes/enhancements/blob/master/keps/0001-kubernetes-enhancement-proposal-process.md#prior-art) on the latter; as well as the [OpenTracing OTEP](https://github.com/opentracing/specification/tree/master/OTEP) process. Massive kudos and thanks to the respective authors and communities for providing excellent prior art 💖
Our OTEP process borrows from the [Rust RFC](https://github.com/rust-lang/rfcs) and [Kubernetes Enhancement Proposal](https://github.com/kubernetes/enhancements) processes, the former also being [very influential](https://github.com/kubernetes/enhancements/blob/master/keps/0001-kubernetes-enhancement-proposal-process.md#prior-art) on the latter; as well as the [OpenTracing OTEP](https://github.com/opentracing/specification/tree/master/rfc_process.md) process. Massive kudos and thanks to the respective authors and communities for providing excellent prior art 💖

[circleci-image]: https://circleci.com/gh/open-telemetry/oteps.svg?style=svg
[circleci-url]: https://circleci.com/gh/open-telemetry/oteps
Expand Down
30 changes: 15 additions & 15 deletions text/0001-telemetry-without-manual-instrumentation.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# (Open) Telemetry Without Manual Instrumentation

**Status:** `approved`

_Cross-language requirements for automated approaches to extracting portable telemetry data with zero source code modification._

## Motivation
Expand All @@ -10,7 +8,7 @@ The purpose of OpenTelemetry is to make robust, portable telemetry a built-in fe

One way to navigate situations like these is with a software layer that adds OpenTelemetry instrumentation to a service without modifying the source code for that service. (In the conventional APM world, these software layers are often called “agents”, though that term is overloaded and ambiguous so we try avoid it in this document.)

### Why “cross-language”?
### Why “cross-language”

Many people have correctly observed that “agent” design is highly language-dependent. This is certainly true, but there are still higher-level “product” objectives for OpenTelemetry that can guide the design choices we make across languages and help users form a consistent impression of what OpenTelemetry provides (and what it does not).

Expand All @@ -25,29 +23,28 @@ Many people have correctly observed that “agent” design is highly language-d
### Requirements

Without further ado, here are a set of requirements for “official” OpenTelemetry efforts to accomplish zero-source-code-modification instrumentation (i.e., “OpenTelemetry agents”) in any given language:

* _Manual_ source code modifications "very strongly discouraged", with an exception for languages or environments that leave no credible alternatives. Any code changes must be trivial and `O(1)` per source file (rather than per-function, etc).
* Licensing must be permissive (e.g., ASL / BSD)
* Packaging must allow vendors to “wrap” or repackage the portable (OpenTelemetry) library into a single asset that’s delivered to customers
* That is, vendors do not want to require users to comprehend both an OpenTelemetry package and a vendor-specific package
* That is, vendors do not want to require users to comprehend both an OpenTelemetry package and a vendor-specific package
* Explicit, whitebox OpenTelemetry instrumentation must interoperate with the “automatic” / zero-source-code-modification / blackbox instrumentation.
* If the blackbox instrumentation starts a Span, whitebox instrumentation must be able to discover it as the active Span (and vice versa)
* Relatedly, there also must be a way to discover and avoid potential conflicts/overlap/redundancy between explicit whitebox instrumentation and blackbox instrumentation of the same libraries/packages
* That is, if a developer has already added the “official” OpenTelemetry plugin for, say, gRPC, then when the blackbox instrumentation effort adds gRPC support, it should *not* “double-instrument” it and create a mess of extra spans/etc
* If the blackbox instrumentation starts a Span, whitebox instrumentation must be able to discover it as the active Span (and vice versa)
* Relatedly, there also must be a way to discover and avoid potential conflicts/overlap/redundancy between explicit whitebox instrumentation and blackbox instrumentation of the same libraries/packages
* That is, if a developer has already added the “official” OpenTelemetry plugin for, say, gRPC, then when the blackbox instrumentation effort adds gRPC support, it should *not* “double-instrument” it and create a mess of extra spans/etc
* From the standpoint of the actual telemetry being gathered, the same standards and expectations (about tagging, metadata, and so on) apply to "whitebox" instrumentation and automatic instrumentation
* The code in the OpenTelemetry package must not take a hard dependency on any particular vendor/vendors (that sort of functionality should work via a plugin or registry mechanism)
* Further, the code in the OpenTelemetry package must be isolated to avoid possible conflicts with the host application (e.g., shading in Java, etc)

* Further, the code in the OpenTelemetry package must be isolated to avoid possible conflicts with the host application (e.g., shading in Java, etc)

### Nice-to-have properties

* Run-time integration (vs compile-time integration)
* Automated and modular testing of individual library/package plugins
* Note that this also makes it easy to test against multiple different versions of any given library
* Note that this also makes it easy to test against multiple different versions of any given library
* A fully pluggable architecture, where plugins can be registered at runtime without requiring changes to the central repo at github.com/open-telemetry
* E.g., for ops teams that want to write a plugin for a proprietary piece of legacy software they are unable to recompile
* E.g., for ops teams that want to write a plugin for a proprietary piece of legacy software they are unable to recompile
* Augemntation of whitebox instrumentation by blackbox instrumentation (or, perhaps, vice versa). That is, not only can the trace context be shared by these different flavors of instrumentation, but even things like in-flight Span objects can be shared and co-modified (e.g., to use runtime interposition to grab local variables and attach them to a manually-instrumented span).


## Trade-offs and mitigations

Approaching a problem this language-specific at the cross-language altitude is intrinsically challenging since "different languages are different" – e.g., in Go there is no way to perform the kind of runtime interpositioning that's possible in Python, Ruby, or even Java.
Expand All @@ -56,23 +53,24 @@ There is also a school of thought that we should only be focusing on the bits an

## Proposal

### What is our desired end state for OpenTelemetry end-users?
### What is our desired end state for OpenTelemetry end-users

To reiterate much of the above:

* First and foremost, **portable OpenTelemetry instrumentation can be installed without manual source code modification**
* There’s one “clear winner” when it comes to portable, automatic instrumentation; just like with OpenTracing and OpenCensus, this is a situation where choice is not necessarily a good thing. End-users who wish to contribute instrumentation plugins should not have their enthusiasm and generosity diluted across competing projects.
* As much as such a thing is possible, consistency across languages
* Broad coverage / “plugin support”
* Broad vendor support for OpenTelemetry
* All other things being equal, get all of these ^^ benefits ASAP!

### What's the basic proposal?
### What's the basic proposal

Given the desired end state, the Datadog tracers seem like the closest-fit, permissively-licensed option out there today. We asked Datadog's leadership whether they would be interested in donating that code to OpenTelemetry, and they were receptive to the idea. (I.e., this would not be a "hard fork" that must be maintained in parallel forever)

### The overarching (technical) process, per-language

* Start with [the Datadog `dd-trace-foo` tracers](https://github.com/DataDog?utf8=✓&q=dd-trace&type=source&language=)
* Start with [the Datadog `dd-trace-foo` tracers](https://github.com/DataDog)
* For each language:
* Fork the Datadog `datadog/dd-trace-foo` repo into a `open-telemetry/auto-instr-foo` OpenTelemetry repo (exact naming TBD)
* In parallel:
Expand Down Expand Up @@ -102,12 +100,14 @@ Each `auto-instr-foo` repository must have at least one [Maintainer](https://git
## Prior art and alternatives

There are many proprietary APM language agents – no need to survey them all here. There is a much smaller list of "APM agents" (or other auto-instrumentation efforts) that are already permissively-licensed OSS. For instance, when we met to discuss options for JVM (longer notes [here](https://docs.google.com/document/d/1ix0WtzB5j-DRj1VQQxraoqeUuvgvfhA6Sd8mF5WLNeY/edit#heading=h.kjctiyv4rxup)), we came away with the following list:

* [Honeycomb's Java beeline](https://github.com/honeycombio/beeline-java)
* [Datadog's Java tracer](https://github.com/datadog/dd-trace-java)
* [Glowroot](https://glowroot.org/)
* [SpecialAgent](https://github.com/opentracing-contrib/java-specialagent)

The most obvious "alternative approach" would be to choose "starting points" independently in each language. This has several problems:

* Higher likelihood of "hard forks": we want to avoid an end state where two projects (the OpenTelemetry version, and the original version) evolve – and diverge – independently
* Higher likelihood of "concept divergence" across languages: while each language presents unique requirements and challenges, the Datadog auto-instrumentation libraries were written by a single organization with some common concepts and architectural requirements (they were also written to be OpenTracing-compatible, which greatly increases our odds of success given the similarities to OpenTelemetry)
* Datadog would also like a uniform strategy here, and this donation requires their consent (unless we want to do a hard fork, which is suboptimal for everyone). So starting with the Datadog libraries in "all but one" (or "all but two", etc) languages makes this less palatable for them
6 changes: 2 additions & 4 deletions text/0002-remove-spandata.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# Remove SpanData

**Status:** `approved`

Remove and replace SpanData by adding span start and end options.

## Motivation
Expand All @@ -24,7 +22,7 @@ I'd like to propose getting rid of SpanData and `tracer.recordSpanData()` and re

## Trade-offs and mitigations

From https://github.com/open-telemetry/opentelemetry-specification/issues/71: If the underlying SDK automatically adds tags to spans such as thread-id, stacktrace, and cpu-usage when a span is started, they would be incorrect for out of band spans as the tracer would not know the difference between in and out of band spans. This can be mitigated by indicating that the span is out of band to prevent attaching incorrect information, possibly with an `isOutOfBand()` option on `startSpan()`.
From <https://github.com/open-telemetry/opentelemetry-specification/issues/71:> If the underlying SDK automatically adds tags to spans such as thread-id, stacktrace, and cpu-usage when a span is started, they would be incorrect for out of band spans as the tracer would not know the difference between in and out of band spans. This can be mitigated by indicating that the span is out of band to prevent attaching incorrect information, possibly with an `isOutOfBand()` option on `startSpan()`.

## Prior art and alternatives

Expand All @@ -38,7 +36,7 @@ There also seems to be some hidden dependency between SpanData and the sampler A

We might want to include attributes as a start option to give the underlying sampler more information to sample with. We also might want to include optional events, which would be for bulk adding events with explicit timestamps.

We will also want to ensure, assuming the span or subtrace is being created in the same process, that the timestamps use the same precision and are monotonic.
We will also want to ensure, assuming the span or subtrace is being created in the same process, that the timestamps use the same precision and are monotonic.

## Related Issues

Expand Down
Loading

0 comments on commit eea5f16

Please sign in to comment.