Use custom data type and custom JSON serialization for traceid #1840
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
Contributes to open-telemetry#1177 1. The TraceID type uses custom data type so that JSON serialization is in hex format instead of base64 (which is the default Protobuf JSON format). Hex format is required by OTLP spec: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/protocol/otlp.md#request SpanID must be also modified similarly. Will be done in a future PR to avoid creating a huge PR. 2. Moved pdata.TraceID to its own file. Note that there is pdata.TraceID which is different from otlp TraceID custom data type. Due to the way packages are structured we need both to keep OTLP generated data types decoupled from pdata data types. The majority of the changes in this commit are simply type changes from []byte to TraceID.
tigrannajaryan
requested review from
bogdandrutu,
dmitryax,
james-bebbington,
nilebox,
owais and
pjanotti
as code owners
tigrannajaryan
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1840 +/- ##
==========================================
- Coverage 91.12% 91.11% -0.01%
==========================================
Files 263 264 +1
Lines 16105 16116 +11
==========================================
+ Hits 14675 14684 +9
- Misses 1003 1004 +1
- Partials 427 428 +1
Continue to review full report at Codecov.
|
jrcamp
reviewed
Sep 23, 2020
processor/samplingprocessor/tailsamplingprocessor/idbatcher/id_batcher_test.go
Show resolved
Hide resolved
bogdandrutu
reviewed
Sep 24, 2020
| @@ -105,17 +105,6 @@ func (td Traces) ResourceSpans() ResourceSpansSlice { | |||
| return newResourceSpansSlice(td.orig) | |||
| } | |||
|
|
|||
| type TraceID []byte | |||
There was a problem hiding this comment.
This is not good, the otlpcommon.TraceID is not exposed, so users cannot set new TraceID. You can have a type TraceID = otlpcommon.TraceID and use the TraceID in the pdata public API.
There was a problem hiding this comment.
That's exactly what I have. See consumer/pdata/traceid.go line 22.
See PR description:
Moved pdata.TraceID to its own file.
bogdandrutu
approved these changes
Sep 24, 2020
|
@tigrannajaryan is there an issue to track the fixes in contrib? This broke the contrib-test in this repo. |
|
No issue, I am already fixing this. This was expected. Will open a PR in a few. |
|
@keitwb fix merged. |
MovieStoreGuy
pushed a commit
to atlassian-forks/opentelemetry-collector
that referenced
this pull request
Nov 11, 2021
* Added Linux-specific detector for the os.description attribute * Generalized OS description detector with placeholder function for unimplemented OSes * Extended osDescription function to *nix OSes based on golang.org/x/sys/unix * Added WithOS resource configuration function to configure all of the OS resource attributes * Implemented osDescription funtion for Windows OS * Improved documentation header for *nix version of the osDescription function * Added support for reading os-release file * Added/updated documentation headers for *nix implementation of osDescription and related functions * Changelog update * Added support for reading macOS version information * Mock approach to test OS description attribute * Extracted common function getFirstAvailableFile to read the first available file from a list of candidates * Upgraded golang.org/x/sys * Changelog update * Fixed wrong function name in documentation header for WithOSDescription * Updated documentation header for platformOSDescription function * Renamed restoreProcessAttributesProviders test helper function The function restoreProcessAttributesProviders was renamed to simply restoreAttributesProviders to better reflect its broader scope, which not only applies to process attribute's providers. * Fixed os_linux.go overriding build tags defined inside the file The suffix on os_linux.go was overriding the build tags already defined in that file. The file was renamed to os_release_unix.go, reflecting the main function defined in the file. For consistency, os_darwin.go was renamed to os_release_darwing.go, as its primary purpose is to also define the osRelease function. * Removed use of discontinued function resource.WithoutBuiltin * Added PR number to changelog entries * Updated go.sum files after run of make lint * Linux implementation: ignore lines with an empty key * Linux implementation: avoid unquoting strings less than two chars * WIP: added tests for Linux support functions * WIP: added tests for charsToString and getFirstAvailableFile functions * Replaced os.CreateTemp with ioutil.TempFile as the former only exists in Go 1.16 * Added unameProvider type to decouple direct reference to unix.Uname function inside Uname() * Added tests for Uname() function * Replaced *os.File with io.Reader in parseOSReleaseFile to ease testing * Added tests for parseOSReleaseFile function * Darwin implementation: added tests for buildOSRelease function * Replaced *os.File with io.Reader in parsePlistFile to ease testing * Darwin implementation: added tests for parsePlistFile function * Type in documentation header for Linux osRelease function * Extracted logic for reading specific registry values into helper functions * Added basic tests for Windows version of platformOSDescription and helper functions * Manually formatted uint64 to strings to have an uniform interface for test assertions * Asserts there's no error when opening registry key for testing Co-authored-by: Robert Pająk <pellared@hotmail.com> * Simplified subtests by using a single test with multiple asserts * go.sum update after running make * Fix typo Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> * WIP: added placeholder implementation of platformOSDescription for unsupported OSes * Fixed typo on osRelease documentation header Co-authored-by: Chris Bandy <bandy.chris@gmail.com> * Fixed typo on test case name for ParsePlistFile tests Co-authored-by: Chris Bandy <bandy.chris@gmail.com> * Linter fix in changelog * go.sum updates after running make * Used strings.Replacer instead of multiple strings.ReplaceAll calls * Optimized implementation of charsToString * Safer temporary file deletion with t.TempDir() * Used t.Cleanup() for safer mocking of runtime providers in OS resource tests * Handled optionality of DisplayVersion registry key. For example, CI machine runs on: Windows Server 2019 Datacenter (1809) [Version 10.0.17763.1999] So, to not add an extra white space due to missing DisplayVersion, this value is checked to be not empty, and only in such case a trailing space is added for that component. * Workaround to handle the case of DisplayVersion registry key not present * Excluded unsupported GOOSes by negation of supported ones * go.sum update after running make Co-authored-by: Anthony Mirabella <a9@aneurysm9.com> Co-authored-by: Robert Pająk <pellared@hotmail.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com> Co-authored-by: Chris Bandy <bandy.chris@gmail.com>
hughesjj
pushed a commit
to hughesjj/opentelemetry-collector
that referenced
this pull request
Apr 27, 2023
Bumps [aquasecurity/trivy-action](https://github.com/aquasecurity/trivy-action) from 0.6.1 to 0.6.2. - [Release notes](https://github.com/aquasecurity/trivy-action/releases) - [Commits](aquasecurity/trivy-action@0.6.1...0.6.2) --- updated-dependencies: - dependency-name: aquasecurity/trivy-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.
Contributes to #1177
The TraceID type uses custom data type so that JSON serialization
is in hex format instead of base64 (which is the default Protobuf JSON
format). Hex format is required by OTLP spec:
https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/protocol/otlp.md#request
SpanID must be also modified similarly. Will be done in a future PR
to avoid creating a huge PR.
Moved pdata.TraceID to its own file. Note that there is pdata.TraceID which
is different from otlp TraceID custom data type. Due to the way packages are
structured we need both to keep OTLP generated data types decoupled from pdata
data types.
The majority of the changes in this commit are simply type changes from
[]byte to TraceID.