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

Build Observability via OpenTelemetry Traces #299

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

Conversation

joshwlewis
Copy link
Member

@joshwlewis joshwlewis commented Oct 6, 2023

This RFC introduces a solution to build observability via OpenTelemetry Traces and the OpenTelemetry File Export format.

Readable

@buildpack-bot
Copy link
Member

Maintainers,

As you review this RFC please queue up issues to be created using the following commands:

/queue-issue <repo> "<title>" [labels]...
/unqueue-issue <uid>

Issues

(none)

Signed-off-by: Josh W Lewis <josh.lewis@salesforce.com>
@joshwlewis joshwlewis marked this pull request as ready for review April 24, 2024 15:41
@joshwlewis joshwlewis changed the title Draft: Build Observability Draft: Build Observability via OpenTelemetry Traces Apr 24, 2024
@joshwlewis joshwlewis changed the title Draft: Build Observability via OpenTelemetry Traces Build Observability via OpenTelemetry Traces Apr 24, 2024
Co-authored-by: Natalie Arellano <narellano@vmware.com>
Signed-off-by: Josh W Lewis <josh.w.lewis@gmail.com>
### Access

The telemetry files should be readable so that they may be analyzed by
the user and/or platform. However, they should be write protected
Copy link
Member

Choose a reason for hiding this comment

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

How will we accomplish this if buildpacks all run as the same user?

Copy link
Member Author

Choose a reason for hiding this comment

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

During build, each buildpack runs synchronously, so we could prep the files/permissions prior to bin/build, so that each buildpack can only write to the expected files.

During detect, many buildpacks will be running at the same time, and all under the same user. I can't think of a way to do block one buildpack from writing to another buildpack's detect telemetry.

To be fair, buildpacks can write to other buildpacks' /layers today. So maybe this section is a little overzealous.

Maybe instead of blocking writes, we say something about the intent - buildpacks shall write only to their own telemetry files.

2) This functionality emits telemetry data only to the build file system. For
`pack` users, the telemetry files are stored in docker volumes on the local
machine. Neither `pack` nor `lifecycle` will "phone home" with telemety data.
3) Neither `pack` nor `lifecycle` collect user-identifiable data (no emails,
Copy link
Member

Choose a reason for hiding this comment

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

Should image/registry names be considered sensitive?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm - tough question. Yeah, I suppose some folks would consider the resulting image name/registry sensitive. I'll update.

- Does `lifecycle` emit files in other places that should be matched?

- What file paths should be used for buildpack telemetry?
- `/layers` paths are not availble during detect, but `detect` tracing is
Copy link
Member

@natalieparellano natalieparellano Apr 26, 2024

Choose a reason for hiding this comment

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

Actually /layers is available during detect, I think it is so we can write /layers/group.toml

Edit: that is from the platform perspective ^^; we would have to think about how to expose a "buildpack layers dir" to buildpacks during detect; the directory structure (/layers/buildpack-id) from the build phase might not work because we assume that only ONE version of a single buildpack-id can detect, whereas multiple versions might be running in parallel during detect.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the location to /layers/#{buildpack-id}.jsonl. That shouldn't require /layers/buildpack-id to exist.

Given multiple versions of a buildpack could be running simultaneously during detect, maybe the version should be included in the filename to prevent multiple buildpacks from writing to the same file simultaneously?

@jkutner
Copy link
Member

jkutner commented Jun 13, 2024

@joshwlewis can you DCO those last commits (and maybe squash em)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants