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

Add make target 'codespell' #863

Merged
merged 11 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .codespellignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
mmaped
ect
10 changes: 10 additions & 0 deletions .codespellrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# https://github.com/codespell-project/codespell
[codespell]
builtin = clear,rare,informal
check-filenames =
check-hidden =
ignore-words = .codespellignore
interactive = 1
skip = .git,go.mod,go.sum,CHANGELOG.md,LICENSES,venv,internal/include/libbpf
uri-ignore-words-list = *
write =
9 changes: 9 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -217,3 +217,12 @@ updates:
schedule:
interval: weekly
day: sunday
- package-ecosystem: pip
directory: /
labels:
- dependencies
- python
- Skip Changelog
schedule:
interval: weekly
day: sunday
14 changes: 14 additions & 0 deletions .github/workflows/codespell.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: codespell
on:
push:
branches:
- main
pull_request:
jobs:
codespell:
runs-on: ubuntu-latest
steps:
- name: Checkout Repo
uses: actions/checkout@v4
- name: Codespell
run: make codespell
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
# VSCode
.vscode/

# Installed by `make codespell`
venv/

otel-go-instrumentation
launcher/
opentelemetry-helm-charts/
Expand Down
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ linters-settings:
- name: constant-logical-expr
disabled: false
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#context-as-argument
# TODO (#3372) reenable linter when it is compatible. https://github.com/golangci/golangci-lint/issues/3280
# TODO (#3372) re-enable linter when it is compatible. https://github.com/golangci/golangci-lint/issues/3280
- name: context-as-argument
disabled: true
arguments:
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http
- Support Go versions from apps defining GOEXPERIMENT. ([#813](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/813))
- Update `net/http` instrumentation to comply with semantic conventions v1.25.0. ([#790](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/790))
- Update existing 3rd party licenses. ([#864](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/864))
- Add make target 'codespell'. ([#863](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/863))

## [v0.12.0-alpha] - 2024-04-10

Expand Down
5 changes: 5 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ git commit
git push <YOUR_FORK> <YOUR_BRANCH_NAME>
```

Additionally, there is a `codespell` target that checks for common
typos in the code. It is not run by default, but you can run it
manually with `make codespell`. It will set up a virtual environment
in `venv` and install `codespell` there.

Open a pull request against the main `opentelemetry-go-instrumentation` repo. Be sure to add the pull
request ID to the entry you added to `CHANGELOG.md`.

Expand Down
39 changes: 38 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ CGO_ENABLED=0
.DEFAULT_GOAL := precommit

.PHONY: precommit
precommit: license-header-check dependabot-generate go-mod-tidy golangci-lint-fix
precommit: license-header-check dependabot-generate go-mod-tidy golangci-lint-fix codespell

# Tools
$(TOOLS):
Expand Down Expand Up @@ -208,3 +208,40 @@ check-clean-work-tree:
echo 'Working tree is not clean, did you forget to run "make precommit", "make generate" or "make offsets"?'; \
exit 1; \
fi

# Virtualized python tools via docker

# The directory where the virtual environment is created.
VENVDIR := venv

# The directory where the python tools are installed.
PYTOOLS := $(VENVDIR)/bin

# The pip executable in the virtual environment.
PIP := $(PYTOOLS)/pip

# The directory in the docker image where the current directory is mounted.
WORKDIR := /workdir

# The python image to use for the virtual environment.
PYTHONIMAGE := python:3.11.3-slim-bullseye

# Run the python image with the current directory mounted.
DOCKERPY := docker run --rm -v "$(CURDIR):$(WORKDIR)" -w $(WORKDIR) $(PYTHONIMAGE)

# Create a virtual environment for Python tools.
$(PYTOOLS):
# The `--upgrade` flag is needed to ensure that the virtual environment is
# created with the latest pip version.
@$(DOCKERPY) bash -c "python3 -m venv $(VENVDIR) && $(PIP) install --upgrade pip"

# Install python packages into the virtual environment.
$(PYTOOLS)/%: $(PYTOOLS)
@$(DOCKERPY) $(PIP) install -r requirements.txt

CODESPELL = $(PYTOOLS)/codespell
$(CODESPELL): PACKAGE=codespell

.PHONY: codespell
codespell: $(CODESPELL)
rockdaboot marked this conversation as resolved.
Show resolved Hide resolved
@$(DOCKERPY) $(CODESPELL)
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ See the [contributing documentation](./CONTRIBUTING.md).
OpenTelemetry Go Automatic Instrumentation is licensed under the terms of the [Apache Software License version 2.0].
See the [license file](./LICENSE) for more details.

Third-party licesnes and copyright notices can be found in the [LICENSES directory](./LICENSES).
Third-party licenses and copyright notices can be found in the [LICENSES directory](./LICENSES).

[OpenTelemetry]: https://opentelemetry.io/
[Go]: https://go.dev/
Expand Down
2 changes: 1 addition & 1 deletion docs/how-it-works.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ The offsets-tracker generates the [offset_results.json](../internal/pkg/inject/o

### Uretprobes

One of the basic requirments of OpenTelemetry spans is to contain start timestamp and end timestamp. Getting those timestamps is possible by placing an eBPF code at the start and the end of the instrumented function. eBPF supports this requirement via uprobes and uretprobes. Uretprobes are used to invoke eBPF code at the end of the function. Unfortunately, uretprobes and Go [do not play well together](https://github.com/golang/go/issues/22008).
One of the basic requirements of OpenTelemetry spans is to contain start timestamp and end timestamp. Getting those timestamps is possible by placing an eBPF code at the start and the end of the instrumented function. eBPF supports this requirement via uprobes and uretprobes. Uretprobes are used to invoke eBPF code at the end of the function. Unfortunately, uretprobes and Go [do not play well together](https://github.com/golang/go/issues/22008).

We overcome this issue by analyzing the target binary and detecting all the return statements in the instrumented functions. We then place a uprobe at the end of each return statement. This uprobe invokes the eBPF code that collects the end timestamp.

Expand Down
2 changes: 1 addition & 1 deletion internal/include/go_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ static __always_inline void stop_tracking_span(struct span_context *sc, struct s
void *ctx = bpf_map_lookup_elem(&tracked_spans_by_sc, sc);
if (ctx == NULL)
{
bpf_printk("stop_tracking_span: cant find span context");
bpf_printk("stop_tracking_span: can't find span context");
return;
}

Expand Down
2 changes: 1 addition & 1 deletion internal/include/otel_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ volatile const u64 attr_type_int64slice;
volatile const u64 attr_type_float64slice;
volatile const u64 attr_type_stringslice;

/* Defintions should mimic structs defined in go.opentelemetry.io/otel/attribute */
/* Definitions should mimic structs defined in go.opentelemetry.io/otel/attribute */

typedef struct go_otel_attr_value {
u64 vtype;
Expand Down
2 changes: 1 addition & 1 deletion internal/include/uprobe.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
struct span_context psc;

// Common flow for uprobe return:
// 1. Find consistend key for the current uprobe context
// 1. Find consistent key for the current uprobe context
// 2. Use the key to lookup for the uprobe context in the uprobe_context_map
// 3. Update the end time of the found span
// 4. Submit the constructed event to the agent code using perf buffer events_map
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ int uprobe_FetchMessage(struct pt_regs *ctx) {
3. Blocking wait for message
4. internal kafka code after blocking
5. Return from FetchMessage
Steps 2-4 are executed in a seperate goroutine from the one the user of the library.
Steps 2-4 are executed in a separate goroutine from the one the user of the library.
*/
void *reader = get_argument(ctx, 1);
void *context_data_ptr = get_Go_context(ctx, 3, 0, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ static __always_inline int inject_kafka_header(void *message, struct kafka_heade

static __always_inline long collect_kafka_attributes(void *message, struct message_attributes_t *attrs, bool collect_topic) {
if (collect_topic) {
// Topic might be globaly set for a writer, or per message
// Topic might be globally set for a writer, or per message
get_go_string_from_user_ptr((void *)(message + message_topic_pos), attrs->topic, sizeof(attrs->topic));
}

Expand Down Expand Up @@ -188,7 +188,7 @@ int uprobe_WriteMessages(struct pt_regs *ctx) {
// https://github.com/segmentio/kafka-go/blob/v0.2.3/message.go#L24C2-L24C6
// 2. the time.Time struct is 24 bytes. This looks to be correct for all the reasnobaly latest versions according to
// https://github.com/golang/go/blame/master/src/time/time.go#L135
// In the future if more libraries will need to get structs sizes we probably want to have simillar
// In the future if more libraries will need to get structs sizes we probably want to have similar
// mechanism to the one we have for the offsets
u16 msg_size = message_time_pos + 8 + 8 + 8;
__builtin_memcpy(current_sc.TraceID, kafka_request->TraceID, TRACE_ID_SIZE);
Expand All @@ -198,7 +198,7 @@ int uprobe_WriteMessages(struct pt_regs *ctx) {
if (i >= msgs_array_len) {
break;
}
// Optionaly collect the topic, and always collect key
// Optionally collect the topic, and always collect key
collect_kafka_attributes(msg_ptr, &kafka_request->msgs[i], !global_topic);
// Generate span id for each message
generate_random_bytes(kafka_request->msgs[i].SpanID, SPAN_ID_SIZE);
Expand All @@ -216,7 +216,7 @@ int uprobe_WriteMessages(struct pt_regs *ctx) {


bpf_map_update_elem(&kafka_events, &key, kafka_request, 0);
// don't need to start tracking the span, as we don't have a context to propagate localy
// don't need to start tracking the span, as we don't have a context to propagate locally
return 0;
}

Expand All @@ -237,6 +237,6 @@ int uprobe_WriteMessages_Returns(struct pt_regs *ctx) {

bpf_perf_event_output(ctx, &events, BPF_F_CURRENT_CPU, kafka_request, sizeof(*kafka_request));
bpf_map_delete_elem(&kafka_events, &key);
// don't need to stop tracking the span, as we don't have a context to propagate localy
// don't need to stop tracking the span, as we don't have a context to propagate locally
return 0;
}
2 changes: 1 addition & 1 deletion internal/pkg/instrumentation/probe/probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func (i *Base[BPFObj, BPFEvent]) Close() error {
// shutdown to avoid leaking system resources.
type UprobeFunc[BPFObj any] func(symbol string, exec *link.Executable, target *process.TargetDetails, obj *BPFObj) ([]link.Link, error)

// Uprobe is an eBPF program that is attached in the entry point and/or the reutrn of a function.
// Uprobe is an eBPF program that is attached in the entry point and/or the return of a function.
type Uprobe[BPFObj any] struct {
// Sym is the symbol name of the function to attach the eBPF program to.
Sym string
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/process/ptrace/ptrace_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func NewTracedProgram(pid int, logger logr.Logger) (*TracedProgram, error) {
tidMap := make(map[int]bool)
retryCount := make(map[int]int)

// iterate over the thread group, until it doens't change
// iterate over the thread group, until it doesn't change
//
// we have tried several ways to ensure that we have stopped all the tasks:
// 1. iterating over and over again to make sure all of them are tracee
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/structfield/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func find[T any](slice *[]*T, f func(*T) bool) *T {
// mergeSorted merges the two sorted slices slice0 and slice1 using the cmp
// function to compare elements.
//
// The cmp function needs to return negative values when a<b, possitive values
// The cmp function needs to return negative values when a<b, positive values
// when a>b, and 0 when a==b.
func mergeSorted[T any](slice0, slice1 []T, cmp func(a, b T) int) []T {
merged := make([]T, 0, len(slice0)+len(slice1))
Expand Down
2 changes: 1 addition & 1 deletion internal/test/e2e/gin/verify.bats
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ SCOPE="go.opentelemetry.io/auto/net/http"
assert_equal "$result" '"/hello-gin"'
}

@test "server :: includes hhttp.response.status_code attribute" {
@test "server :: includes http.response.status_code attribute" {
result=$(server_span_attributes_for ${SCOPE} | jq "select(.key == \"http.response.status_code\").value.intValue")
assert_equal "$result" '"200"'
}
Expand Down
2 changes: 1 addition & 1 deletion internal/test/e2e/nethttp/verify.bats
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ SCOPE="go.opentelemetry.io/auto/net/http"
assert_equal "$result" '"/hello/42"'
}

@test "server :: includes hhttp.response.status_code attribute" {
@test "server :: includes http.response.status_code attribute" {
result=$(server_span_attributes_for ${SCOPE} | jq "select(.key == \"http.response.status_code\").value.intValue")
assert_equal "$result" '"200"'
}
Expand Down
2 changes: 1 addition & 1 deletion internal/test/e2e/nethttp_custom/verify.bats
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ SCOPE="go.opentelemetry.io/auto/net/http"
assert_equal "$result" '"/hello"'
}

@test "server :: includes hhttp.response.status_code attribute" {
@test "server :: includes http.response.status_code attribute" {
result=$(server_span_attributes_for ${SCOPE} | jq "select(.key == \"http.response.status_code\").value.intValue")
assert_equal "$result" '"200"'
}
Expand Down
2 changes: 1 addition & 1 deletion internal/tools/inspect/inspector.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (i *Inspector) AddManifest(manifest Manifest) error {

goVer := manifest.Application.GoVerions
if goVer == nil {
// Passsing nil to newBuilder will mean the application is built with
// Passing nil to newBuilder will mean the application is built with
// the latest version of Go.
b := newBuilder(i.log, i.client, nil)
for _, ver := range manifest.Application.Versions {
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
codespell==2.2.4
Loading