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 example for OTLP logging via stdout and k8s #547

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

zeitlinger
Copy link
Member

@zeitlinger zeitlinger commented Dec 16, 2024

@zeitlinger zeitlinger self-assigned this Dec 16, 2024
@zeitlinger zeitlinger marked this pull request as ready for review December 18, 2024 13:29
@zeitlinger zeitlinger requested review from a team as code owners December 18, 2024 13:29
@zeitlinger
Copy link
Member Author

@trask can you take a look?

exporters: [ otlphttp/metrics ]
logs/raw_otlpjson:
receivers: [ filelog/otlp-json-logs ]
# (i) no need for processors before the otlpjson connector
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does (i) mean?

Comment on lines +5 to +7
./build.sh
k3d cluster create jsonlogging || k3d cluster start jsonlogging
k3d image import -c jsonlogging dice:1.1-SNAPSHOT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this file already has a dependency on the dice image, does it make sense to just inline the one line here?


- a Java application that uses the experimental
[experimental-otlp/stdout](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#in-development-exporter-selection) logs exporter
- a OTel collector configuration that uses the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- a OTel collector configuration that uses the
- an OpenTelemetry collector configuration that uses the

Comment on lines +23 to +25
The OTel Collector pipeline:

![OTel Collector Pipeline](otel-collector-otlpjson-pipeline.png)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The OTel Collector pipeline:
![OTel Collector Pipeline](otel-collector-otlpjson-pipeline.png)
The OpenTelemetry Collector pipeline:
![OpenTelemetry Collector Pipeline](otel-collector-otlpjson-pipeline.png)

repository: grafana/oats
ref: bc2f08c5e55114234cece216290f1580a75a6032
path: oats
- name: Set up Go
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(sorry, just a nitpicky preference I have)

Suggested change
- name: Set up Go
- name: Set up Go

with:
go-version: '1.23'
cache-dependency-path: oats/go.sum
- name: Run acceptance tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- name: Run acceptance tests
- name: Run acceptance tests

cache-dependency-path: oats/go.sum
- name: Run acceptance tests
run: .github/scripts/run-acceptance-tests.sh
- name: upload log file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- name: upload log file
- name: upload log file

# renovate: datasource=github-releases depName=opentelemetry-java-instrumentation packageName=open-telemetry/opentelemetry-java-instrumentation
ENV OPENTELEMETRY_JAVA_INSTRUMENTATION_VERSION=v2.10.0

ADD build/libs/*SNAPSHOT.jar ./app.jar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think about adding the ../gradlew assemble here, and then I think could remove it from (the global) run-acceptance-tests.sh file?

Comment on lines +38 to +49
"customManagers": [
{
"customType": "regex",
"description": "Update _VERSION variables in Dockerfiles",
"fileMatch": [
"(^|/|\\.)Dockerfile$",
"(^|/)Dockerfile\\.[^/]*$"
],
"matchStrings": [
"# renovate: datasource=(?<datasource>[a-z-]+?)(?: depName=(?<depName>.+?))? packageName=(?<packageName>.+?)(?: versioning=(?<versioning>[a-z-]+?))?\\s(?:ENV|ARG) .+?_VERSION=(?<currentValue>.+?)\\s"
]
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe rename to run-oats-tests.sh? (and rename workflow to oats-tests.yml)

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

Successfully merging this pull request may close these issues.

3 participants