Adds profiles as another signal #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Jul 6, 2023
This was referenced Jul 27, 2023
jsuereth
pushed a commit
to open-telemetry/oteps
that referenced
this pull request
Feb 23, 2024
This is second version of the Profiling Data Model OTEP. After [we've gotten feedback from the greater OTel community](#237) we went back to the drawing board and came up with a new version of the data model. The main difference between the two versions is that the new version is more similar to the original pprof format, which makes it easier to understand and implement. It also has better performance characteristics. We've also incorporated a lot of the feedback we've gotten on the first PR into this OTEP. Some minor details about the data model are still being discussed and will be flushed out in the future OTEPs. We intend to finalize these details after doing experiments with early versions of working client + collector + backend implementations and getting feedback from the community. The goal of this OTEP is to provide a solid foundation for these experiments. So far we've done a number of things to validate it: * we've written a new profiles proto described in this OTEP * we've documented decisions made along the way in a [decision log](https://github.com/open-telemetry/opentelemetry-proto-profile/blob/main/opentelemetry/proto/profiles/v1/decision-log.md) * we've done benchmarking to refine the data representation (see Benchmarking section in a [collector PR](petethepig/opentelemetry-collector#1)) * diff between original pprof and the new proto: [link](open-telemetry/opentelemetry-proto-profile@2cf711b...petethepig:opentelemetry-proto:pprof-experiments#diff-9cb689ea05ecfd2edffc39869eca3282a3f2f45a8e1aa21624b452fa5362d1d2) We're seeking feedback and hoping to get this approved. --- For (a lot) more details, see: * [OTel Profiling SIG Meeting Notes](https://docs.google.com/document/d/19UqPPPlGE83N37MhS93uRlxsP1_wGxQ33Qv6CDHaEp0/edit) --------- Co-authored-by: Juraci Paixão Kröhling <juraci.github@kroehling.de> Co-authored-by: Christos Kalkanis <christos.kalkanis@elastic.co> Co-authored-by: Felix Geisendörfer <felix@felixge.de> Co-authored-by: Reiley Yang <reyang@microsoft.com>
petethepig
pushed a commit
that referenced
this pull request
May 13, 2024
To resolve the govulncheck reports: ``` Vulnerability #1: GO-2023-1987 Large RSA keys can cause high CPU usage in crypto/tls More info: https://pkg.go.dev/vuln/GO-2023-1987 Standard library Found in: crypto/tls@go1.19.11 Fixed in: crypto/tls@go1.21rc4 Example traces found: Error: #1: service/internal/proctelemetry/config.go:299:27: proctelemetry.initOTLPgRPCExporter calls otlpmetricgrpc.New, which eventually calls tls.Conn.Handshake Error: open-telemetry#2: service/internal/proctelemetry/config.go:156:39: proctelemetry.InitPrometheusServer calls http.Server.ListenAndServe, which eventually calls tls.Conn.HandshakeContext Error: open-telemetry#3: service/service.go:251:36: service.buildResource calls uuid.NewRandom, which eventually calls tls.Conn.Read Error: open-telemetry#4: service/config.go:35:13: service.Config.Validate calls fmt.Printf, which eventually calls tls.Conn.Write Error: #5: service/telemetry/telemetry.go:32:28: telemetry.Telemetry.Shutdown calls trace.TracerProvider.Shutdown, which eventually calls tls.Dialer.DialContext ``` https://github.com/open-telemetry/opentelemetry-collector/actions/runs/5753675727/job/15597394973?pr=8144
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is my attempt at bringing profiling signal to the collector. It depends on proto defenitions from open-telemetry/opentelemetry-proto repo.
Most of this code is boilerplate, there's a handful of interesting files:
profiles.protoinopentelemetry-protorepo (link)pdata/internal/cmd/pdatagen/internal/pprofile_package.goconnects everything to pdata and needs to be updated manually when proto changes (link)pdata/pprofile/json.goimplements json unmarshaling and needs to be updated manually when proto changes (link)Workflow I use to test this right now:
make genprototo generate go bindingsrsync -av ../opentelemetry-proto/ ${OPENTELEMETRY_PROTO_SRC_DIR})make genpdatato generate more pdata bindingsmake otelcorecolto verify that collector compiles./bin/otelcorecol_darwin_arm64 --config=file:tmp/config.yaml(config is here)curl -i -X POST http://localhost:4318/v1/profiles -H 'Content-Type: application/json' -d @profile.json(profile.json is here)To run benchmarks:
Clone this repo, make sure you're on the right branch (
profiling). Run the following command:If you want to make changes to the proto:
../opentelemetry-proto/)git checkout 622c1658673283102a9429109185615bfcfaa78e../opentelemetry-proto/make genprototo generate protobuf bindingsmake genpdatato generate pdata bindingsYou can now run benchmarks again:
Benchmarks live in
/pdata/pprofiledirectory. The files that you might want to look at are:/pdata/pprofile/conversion.go/pdata/pprofile/benchmark_test.go