Improve handling of status code/message (#51) #70

owais · 2019-06-28T10:13:26Z

Improved how translators handling mapping status codes and messages to and from OC

translator/trace/README.md documents the new behaviour: https://github.com/open-telemetry/opentelemetry-service/pull/70/files#diff-cf653ae20e304fa43f02c3a37a01a3cd

codecov-io · 2019-06-28T11:25:21Z

Codecov Report

❗ No coverage uploaded for pull request base (master@e32ae89). Click here to learn what that means.
The diff coverage is 91.97%.

@@            Coverage Diff            @@
##             master      #70   +/-   ##
=========================================
  Coverage          ?   71.91%           
=========================================
  Files             ?       94           
  Lines             ?     6145           
  Branches          ?        0           
=========================================
  Hits              ?     4419           
  Misses            ?     1499           
  Partials          ?      227

Impacted Files	Coverage Δ
...sor/tracesamplerprocessor/tracesamplerprocessor.go	`100% <ø> (ø)`
translator/trace/spandata/protospan_to_spandata.go	`59.88% <ø> (ø)`
receiver/prometheusreceiver/factory.go	`77.5% <ø> (ø)`
receiver/zipkinreceiver/factory.go	`100% <ø> (ø)`
translator/trace/grpc_http_mapper.go	`0% <0%> (ø)`
translator/trace/zipkin/zipkinv1_to_protospan.go	`90.14% <100%> (ø)`
...anslator/trace/jaeger/protospan_to_jaegerthrift.go	`82.41% <100%> (ø)`
translator/trace/zipkin/status_code.go	`100% <100%> (ø)`
...lator/trace/zipkin/zipkinv1_thrift_to_protospan.go	`78.06% <100%> (ø)`
...ranslator/trace/jaeger/protospan_to_jaegerproto.go	`76.22% <100%> (ø)`
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e32ae89...2fba65b. Read the comment docs.

tigrannajaryan

LGTM, I already reviewed this in details on our internal repo.

tigrannajaryan · 2019-06-28T12:59:23Z

translator/trace/jaeger/jaegerthrift_to_protospan.go

@@ -1,4 +1,4 @@
-// Copyright 2019, OpenTelemetry Authors
+// Copyright 2018, OpenTelemetry Authors


Unnecessary change:

Suggested change

// Copyright 2018, OpenTelemetry Authors

// Copyright 2019, OpenTelemetry Authors

tigrannajaryan · 2019-06-28T12:59:35Z

translator/trace/jaeger/jaegerthrift_to_protospan_test.go

@@ -1,4 +1,4 @@
-// Copyright 2019, OpenTelemetry Authors
+// Copyright 2018, OpenTelemetry Authors


tigrannajaryan · 2019-06-28T13:00:13Z

translator/trace/protospan_translation.go

@@ -1,4 +1,4 @@
-// Copyright 2019, OpenTelemetry Authors
+// Copyright 2018, OpenTelemetry Authors


Fix here and elsewhere too.

flands

LGTM

codefromthecrypt · 2019-06-30T09:59:34Z

translator/trace/README.md

+* `code` should be added as a `census.status_code` tag.
+* `message` should be added as a `census.status_description` tag.
+
+In addition to this, if the OC status field represents a non-OK status, then a tag with the key `error` and value `true` should be added to the Zipkin span if one is not already present. 


please do not do this. zipkin has always regarded the "error" true an anti pattern. instrumentation have been known to set false nonsense value

the value of the "error" tag should be the best available message ex status code if a text variant is not available. empty message is ok and better than true.

pjanotti

Just change zipkin behavior per maintainer's request, otherwise LGTM.

owais · 2019-07-01T14:41:41Z

Updated.

Improved how translators handling mapping status codes and messages to and from OC translator/trace/README.md documents the new behaviour

codefromthecrypt · 2019-07-02T00:15:12Z

translator/trace/README.md

+* `code` should be added as a `census.status_code` tag.
+* `message` should be added as a `census.status_description` tag.
+
+In addition to this, if the OC status field represents a non-OK status, then a tag with the key `error` should be added and set to the same value as that of the status message falling back to status code when status message is not available.


* Initial commit * Add CODEOWNERS file (#2) * Add CODEOWNERS file * Update CODEOWNERS * Moved from github.com/observatorium/opentelemetry-collector-builder (#3) Signed-off-by: Juraci Paixão Kröhling <[email protected]> * fixed panics (#6) Signed-off-by: Joe Elliott <[email protected]> * Replace master with main in CI and mergify files (#8) Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Bump to OpenTelemetry Collector 0.20.0 (#10) Closes #9 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Explicitly enable Go modules in quickstart instructions (#13) * Update to collector v0.21.0 (#17) Fixes #16 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Update to collector v0.22.0 (#19) * Download go modules before building (#20) Fixes #14 * Add version command (#25) Signed-off-by: Ashmita Bohara <[email protected]> * Pass errors from cobra Execute back to main for correct exit code (#28) * pass errors from cobra execute back to main * print the error * Update to collector v0.23.0 (#27) * Generate a warning if the builder and collector base version mismatch (#30) * Generate a warning if the builder and collector base version mismatch * Show current default version in the warning message * Update to OpenTelemetry Collector 0.24.0 * Don't use %w formatting with log.Fatal (#35) * Update to OpenTelemetry Collector 0.25.0 (#36) Signed-off-by: Serge Catudal <[email protected]> * Update to 0.26.0 and update BuildInfo (#39) * Sync build and CI Go versions at latest 1.16 (#34) * Sync build and CI Go versions at latest 1.16 * Run go mod tidy * Set go binary to use in the compilation phase in tests Signed-off-by: Juraci Paixão Kröhling <[email protected]> Co-authored-by: Juraci Paixão Kröhling <[email protected]> * Add option to generate go code only (no compile) (#40) * Issue#24 Add option to generate go code only (no compile) * Update cmd/root.go logging Suggested by @jpkkrohling Co-authored-by: Juraci Paixão Kröhling <[email protected]> * remove verbose help .. created by corba * suggestion by jpkrohling to keep generateandcompile * lint error: remove unused var * reword cmd option and add back help message for default Co-authored-by: Juraci Paixão Kröhling <[email protected]> * Don't reuse exec.Cmd (#42) * Update to OpenTelemetry Collector 0.27.0 (#43) * Add CI Badge (#47) * Update to Collector v0.28.0 (#49) * Update to Collector v0.28.0 Closes #48 Addresses the breaking API change in #3163, besides the usual version number changes. Signed-off-by: Fangyi Zhou <[email protected]> * Use `go mod tidy` instead of `go mod download` It appears that this magically resolves the go.mod file issue. https://stackoverflow.com/questions/67203641/missing-go-sum-entry-for-module-providing-package-package-name Signed-off-by: Fangyi Zhou <[email protected]> * Account for go mod download in go1.17 not updating go.sum (#50) * Update to collector v0.29.0 (#54) * Update replaces.builder.yaml * Update nocore.builder.yaml * Update config.go * Update README.md * Update main.go * Update to collector v0.30.0 (#57) * cmd: fix module flag default value to github.com/open-telemetry (#58) Signed-off-by: Koichi Shiraishi <[email protected]> * Update to collector v0.31.0 (#60) * Update to v0.33.0 (#62) Signed-off-by: Anthony J Mirabella <[email protected]> * Add excludes support to generated go.mod (#63) Signed-off-by: Anthony J Mirabella <[email protected]> Co-authored-by: Juraci Paixão Kröhling <[email protected]> * Small cleanup for the builder files (#64) Signed-off-by: Bogdan Drutu <[email protected]> * Support building with Go 1.17 (#66) * Support building with Go 1.17 Fixes #65 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Update workflows to use Go 1.17 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Add gosec exceptions for exec.Command Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Update to OpenTelemetry core 0.34.0 (#68) Fixes #67 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Upgrade to OpenTelemetry Collector 0.35.0 (#70) Signed-off-by: Fangyi Zhou <[email protected]> * Upgrade to OpenTelemetry Collector 0.36.0 (#76) * Generate custom service code for Windows (#75) * update main to include windows service code * use main version from tag 0.35.0 * update main function * align with upstream v0.36.0 tag * dummy change to trigger build * Revert "dummy change to trigger build" This reverts commit 629d499461da2d2c240bf1e495b5fe0558e3547f. * Remove Core from Module type (#77) Fixes #15 Signed-off-by: yugo-horie <[email protected]> * release 0.37.0 (#78) * release 0.37.0 * update use of NewCommand * Move builder to subdirectory Signed-off-by: Juraci Paixão Kröhling <[email protected]> Co-authored-by: Bogdan Drutu <[email protected]> Co-authored-by: Bogdan Drutu <[email protected]> Co-authored-by: Joe Elliott <[email protected]> Co-authored-by: Eric Yang <[email protected]> Co-authored-by: Brian Gibbins <[email protected]> Co-authored-by: Ashmita <[email protected]> Co-authored-by: Fangyi Zhou <[email protected]> Co-authored-by: Shaun Creary <[email protected]> Co-authored-by: Patryk Małek <[email protected]> Co-authored-by: Serge Catudal <[email protected]> Co-authored-by: Aaron Stone <[email protected]> Co-authored-by: Patryk Małek <[email protected]> Co-authored-by: Aaron Stone <[email protected]> Co-authored-by: Kelvin Lo <[email protected]> Co-authored-by: Himanshu <[email protected]> Co-authored-by: Y.Horie <[email protected]> Co-authored-by: Koichi Shiraishi <[email protected]> Co-authored-by: Anthony Mirabella <[email protected]> Co-authored-by: Cal Loomis <[email protected]> Co-authored-by: alrex <[email protected]>

Signed-off-by: Fangyi Zhou <[email protected]>

- also add implementation of AlwaysSampleSampler and NeverSampleSampler

* Update deps to v0.19.0 * Remove deprecated signalfxcorrelation exporter

owais requested review from bogdandrutu, flands, pjanotti, songy23 and tigrannajaryan as code owners June 28, 2019 10:13

owais changed the title ~~Improve handling of status code/message (#51)~~ WIP: Improve handling of status code/message (#51) Jun 28, 2019

owais changed the title ~~WIP: Improve handling of status code/message (#51)~~ Improve handling of status code/message (#51) Jun 28, 2019

tigrannajaryan previously approved these changes Jun 28, 2019

View reviewed changes

owais dismissed tigrannajaryan’s stale review via 8ff86dc June 28, 2019 13:18

flands assigned owais Jun 28, 2019

flands added this to the 0.1.0 milestone Jun 28, 2019

flands previously approved these changes Jun 28, 2019

View reviewed changes

owais dismissed flands’s stale review via 6f851e0 June 29, 2019 11:28

owais mentioned this pull request Jun 30, 2019

Status is dropped when translating spans to Jaeger TChannel spans census-instrumentation/opencensus-service#586

Open

codefromthecrypt reviewed Jun 30, 2019

View reviewed changes

pjanotti previously approved these changes Jul 1, 2019

View reviewed changes

owais dismissed pjanotti’s stale review via 9b9af91 July 1, 2019 14:37

pjanotti previously approved these changes Jul 1, 2019

View reviewed changes

tigrannajaryan previously approved these changes Jul 1, 2019

View reviewed changes

Improve handling of status code/message (#51)

2fba65b

Improved how translators handling mapping status codes and messages to and from OC translator/trace/README.md documents the new behaviour

owais dismissed stale reviews from tigrannajaryan and pjanotti via 2fba65b July 1, 2019 22:23

tigrannajaryan approved these changes Jul 1, 2019

View reviewed changes

pjanotti approved these changes Jul 1, 2019

View reviewed changes

pjanotti merged commit 21339fb into open-telemetry:master Jul 1, 2019

codefromthecrypt reviewed Jul 2, 2019

View reviewed changes

owais mentioned this pull request Jul 3, 2019

REQUEST: New membership for owais open-telemetry/community#146

Closed

6 tasks

owais mentioned this pull request Sep 7, 2019

Nominating @owais as approver for Agent and Collector open-telemetry/community#172

Merged

owais deleted the status-translation branch February 19, 2020 00:43

jpkrohling mentioned this pull request Oct 28, 2021

Move opentelemetry-collector-builder to core #4286

Merged

bogdandrutu mentioned this pull request Oct 28, 2021

Move opentelemetry-collector-builder to core #4291

Merged

bogdandrutu pushed a commit that referenced this pull request Oct 28, 2021

Upgrade to OpenTelemetry Collector 0.35.0 (#70)

eb82ae4

Signed-off-by: Fangyi Zhou <[email protected]>

MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021

Add Sampler API (open-telemetry#70)

a54a8a2

- also add implementation of AlwaysSampleSampler and NeverSampleSampler

mphillips-infoblox mentioned this pull request Nov 19, 2021

Tracing fixes for error span infobloxopen/atlas-app-toolkit#297

Merged

hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023

Update deps to v0.19.0 (open-telemetry#70)

46eb46a

* Update deps to v0.19.0 * Remove deprecated signalfxcorrelation exporter

Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024

Clarify ABI requirements for the SDK vendor hooks (open-telemetry#70)

8683fbf

swiatekm pushed a commit to swiatekm/opentelemetry-collector that referenced this pull request Oct 9, 2024

Remove invalid crd fields (open-telemetry#70)

a9e1c6f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve handling of status code/message (#51) #70

Improve handling of status code/message (#51) #70

owais commented Jun 28, 2019 •

edited

Loading

codecov-io commented Jun 28, 2019 •

edited

Loading

tigrannajaryan left a comment

tigrannajaryan Jun 28, 2019

tigrannajaryan Jun 28, 2019

tigrannajaryan Jun 28, 2019

flands left a comment

codefromthecrypt Jun 30, 2019

pjanotti left a comment

owais commented Jul 1, 2019

codefromthecrypt Jul 2, 2019

		@@ -1,4 +1,4 @@
		// Copyright 2019, OpenTelemetry Authors
		// Copyright 2018, OpenTelemetry Authors

Improve handling of status code/message (#51) #70

Improve handling of status code/message (#51) #70

Conversation

owais commented Jun 28, 2019 • edited Loading

codecov-io commented Jun 28, 2019 • edited Loading

Codecov Report

tigrannajaryan left a comment

Choose a reason for hiding this comment

tigrannajaryan Jun 28, 2019

Choose a reason for hiding this comment

tigrannajaryan Jun 28, 2019

Choose a reason for hiding this comment

tigrannajaryan Jun 28, 2019

Choose a reason for hiding this comment

flands left a comment

Choose a reason for hiding this comment

codefromthecrypt Jun 30, 2019

Choose a reason for hiding this comment

pjanotti left a comment

Choose a reason for hiding this comment

owais commented Jul 1, 2019

codefromthecrypt Jul 2, 2019

Choose a reason for hiding this comment

owais commented Jun 28, 2019 •

edited

Loading

codecov-io commented Jun 28, 2019 •

edited

Loading