[receiver/pprof] initial commit to convert google/pprof to OTel profiles #42843
Conversation
Follow up to open-telemetry#40548 TODO: - [ ] verify result - [ ] more tests Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
| const ( | ||
| // noAttrUnit is a helper to indicate that no | ||
| // unit is associated to this Attribute. | ||
| noAttrUnit = int32(-1) |
There was a problem hiding this comment.
Can this be a zero? I think with the "zero index == null" convention we've been trying to avoid having to use -1 as a special value.
There was a problem hiding this comment.
noAttrUnit is just an conversion internal helper and will not be seen in the resulting OTel profiles output. As every positive value can indicate a unit, i needed something to indicate that no unit is set for this attribute.
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
|
Thank you for this work. A nit on the test code, and a few notes on the TODOs in the code. Do you want to address the TODOs in this PR or deal with them in other PRs? |
| lastFunctionTableIdx int32 | ||
| } | ||
|
|
||
| func convertPprofToPprofile(src *profile.Profile) (*pprofile.Profiles, error) { |
There was a problem hiding this comment.
Not a blocker for this PR, but if we are willing to add a pprof exporter, we could consider moving this to a pprof translator package
There was a problem hiding this comment.
maybe @MovieStoreGuy or @atoulme can answer the question on why they chose to create receiver/pprofreceiver over a translator package.
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
@atoulme the TODOs were a reminder for myself to discuss the topic of the id fields from pprof with the Profiling SIG. As this is not relevant for this PR, I have removed them with 00333f2. |
|
|
||
| // getIdxForFunction returns the corresponding index for the function. | ||
| // If the function does not yet exist in the cache, it will be adedd. | ||
| func getIdxForFunction(lts lookupTables, name, systemName, fileName string, startLine int64) int32 { |
There was a problem hiding this comment.
The methods below mutate the lookupTables struct, shouldn't this be a reference instead? What about moving them to be a lookupTable's method instead?
| func getIdxForFunction(lts lookupTables, name, systemName, fileName string, startLine int64) int32 { | |
| func (lts *lookupTables) getIdxForFunction(name, systemName, fileName string, startLine int64) int32 { |
Co-authored-by: Roger Coll <roger.coll@elastic.co>
Co-authored-by: Roger Coll <roger.coll@elastic.co>
Co-authored-by: Roger Coll <roger.coll@elastic.co>
|
friendly ping for feedback at @MovieStoreGuy and @atoulme |
#### Description Implement scraper for Profiles. This will help implement receiver parts like open-telemetry/opentelemetry-collector-contrib#42843. --------- Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
5 participants
Follow up to #40548
FYI: @atoulme @felixge @aalexand
TODO: