Proto update to latest to support arrays and maps #1339

obecny · 2020-07-22T18:15:43Z

Which problem is this PR solving?

Fixes Add support for arrays and maps for attribute values for OpenTelemetry Collector Exporter #1190

Short description of the changes

Updating to latest proto adding support for arrays and maps,
working fine with latest 0.6 opentelemetry-collector

…o proto_update

codecov · 2020-07-22T18:20:18Z

Codecov Report

Merging #1339 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1339      +/-   ##
==========================================
+ Coverage   93.17%   93.18%   +0.01%     
==========================================
  Files         149      149              
  Lines        4218     4228      +10     
  Branches      867      869       +2     
==========================================
+ Hits         3930     3940      +10     
  Misses        288      288

Impacted Files	Coverage Δ
...ages/opentelemetry-exporter-collector/src/types.ts	`100.00% <ø> (ø)`
.../opentelemetry-exporter-collector/src/transform.ts	`96.70% <100.00%> (+0.40%)`	⬆️

dyladan

This looks fine, but you've done significantly more than just update the protos. Can you update the title to reflect that (the title goes into the changelog directly)?

…n thye are ignored and when having custom they are ignored too

mwear

LGTM

obecny · 2020-07-27T17:28:04Z

@open-telemetry/javascript-approvers ^^

mayurkale22 · 2020-07-27T17:50:56Z

examples/collector-exporter-node/start.js

@@ -1,11 +1,13 @@
 'use strict';

 const opentelemetry = require('@opentelemetry/api');
+// const { ConsoleLogger,  LogLevel} = require('@opentelemetry/core');


Can we remove this?

I'm keeping them for easier debugging only so that I can see which option has been chosen how the spans looks like etc. and recently have been using this quite heavily. Although not strong against not removing if you think they shouldn't be. But this is just an example mainly for devs so I guess it might be useful to have it in one place. WDYT ?

@mayurkale22

opentelemetry-js/examples/collector-exporter-node/start.js

Line 10 in 905a5d6

// logger: new ConsoleLogger(LogLevel.DEBUG),

and here

opentelemetry-js/examples/collector-exporter-node/start.js

Line 11 in 905a5d6

// headers: {

Think this is fine since it is an example.

@mayurkale22 ok to merge?

dyladan · 2020-07-27T17:57:39Z

Think this can be merged after mayur's comment is resolved

* upstream/master: fix: add missing grpc-js index (open-telemetry#1358) chore: 0.10.0 release proposal (open-telemetry#1345) Proto update to latest to support arrays and maps (open-telemetry#1339) feat: add OTEL_LOG_LEVEL env var (open-telemetry#974) Add nodejs sdk package (open-telemetry#1187) feat: add baggage support to the opentracing shim (open-telemetry#918) Removing default span attributes (open-telemetry#1342)

* feat: graceful shutdown for tracing and metrics * fix: wording in test case * fix: typo * fix meterprovider config to use bracket notation Co-authored-by: Daniel Dyla <[email protected]> * fix meterprovider config to use bracket notation Co-authored-by: Daniel Dyla <[email protected]> * fix: add callbacks to shutdown methods * fix: merge conflict * simplify meter shutdown code Co-authored-by: Daniel Dyla <[email protected]> * fix: fix one-liner * private function name style fix Co-authored-by: Daniel Dyla <[email protected]> * fix: naming of private member variables * fix: graceful shutdown now works in browser * fix: window event listener will trigger once * fix: modify global shutdown helper functions * fix: remove callback from remove listener args * fix: change global shutdown function names and simplify functionality * fix: add rest of function refactoring and simplification * fix: remove unintended code snippet * fix: refactor naming of listener cleanup function and fix sandbox issue * fix: make global shutdown cleanup local * fix: change interval of MeterProvider collection to ensure it does not trigger through clock * chore: removing _cleanupGlobalShutdownListeners * fix: remove unnecesary trace provider member function * Removing default span attributes (#1342) * refactor(opentelemetry-tracing): removing default span attributes Signed-off-by: Aravin Sivakumar <[email protected]> * refactor(opentelemetry-tracing): removing default span attributed from tracer object Signed-off-by: Aravin Sivakumar <[email protected]> * refactor(opentelemetry-tracing): removing accidental add to package.json Signed-off-by: Aravin Sivakumar <[email protected]> * refactor(opentelemetry-tracing): removing redundant test and fixing suggestions by Shawn and Daniel Signed-off-by: Aravin Sivakumar <[email protected]> * feat: add baggage support to the opentracing shim (#918) Co-authored-by: Mayur Kale <[email protected]> * Add nodejs sdk package (#1187) Co-authored-by: Naseem <[email protected]> Co-authored-by: legendecas <[email protected]> Co-authored-by: Mark Wolff <[email protected]> Co-authored-by: Matthew Wear <[email protected]> * feat: add OTEL_LOG_LEVEL env var (#974) * Proto update to latest to support arrays and maps (#1339) * chore: 0.10.0 release proposal (#1345) * fix: add missing grpc-js index (#1358) * chore: 0.10.1 release proposal (#1359) * feat(api/context-base): change compile target to es5 (#1368) * Feat: Make ID generator configurable (#1331) Co-authored-by: Daniel Dyla <[email protected]> * fix: require grpc-js instead of grpc in grpc-js example (#1364) Co-authored-by: Bartlomiej Obecny <[email protected]> * chore(deps): update all non-major dependencies (#1371) * chore: bump metapackage dependencies (#1383) * chore: 0.10.2 proposal (#1382) * fix: remove unnecesary trace provider member function * refactor(metrics): distinguish different aggregator types (#1325) Co-authored-by: Daniel Dyla <[email protected]> * Propagate b3 parentspanid and debug flag (#1346) * feat: Export MinMaxLastSumCountAggregator metrics to the collector as Summary (#1320) Co-authored-by: Daniel Dyla <[email protected]> * feat: Collector Metric Exporter for the Web (#1308) Co-authored-by: Daniel Dyla <[email protected]> * Fix issues in TypeScript getting started example code (#1374) Co-authored-by: Daniel Dyla <[email protected]> * chore: deploy canary releases (#1384) * fix: protos pull * fix: address marius' feedback * chore: deleting removeAllListeners from prometheus, fixing tests, cleanu of events when using shutdown notifier * fix: add documentation and cleanup code * fix: remove async label from shutdown and cleanup test case * fix: update controller collect to return promise * fix: make downsides of disabling graceful shutdown more apparent Co-authored-by: Daniel Dyla <[email protected]> Co-authored-by: Bartlomiej Obecny <[email protected]> Co-authored-by: Aravin <[email protected]> Co-authored-by: Ruben Vargas Palma <[email protected]> Co-authored-by: Mayur Kale <[email protected]> Co-authored-by: Naseem <[email protected]> Co-authored-by: legendecas <[email protected]> Co-authored-by: Mark Wolff <[email protected]> Co-authored-by: Matthew Wear <[email protected]> Co-authored-by: Naseem <[email protected]> Co-authored-by: Mark Wolff <[email protected]> Co-authored-by: Cong Zou <[email protected]> Co-authored-by: Reginald McDonald <[email protected]> Co-authored-by: WhiteSource Renovate <[email protected]> Co-authored-by: srjames90 <[email protected]> Co-authored-by: David W <[email protected]> Co-authored-by: Mick Dekkers <[email protected]>

obecny added 19 commits July 10, 2020 14:38

feat: adding proto over http for collector exporter

99d8854

chore: lint

6638c32

chore: lint

dbdd707

chore: fixing transport type in example

32a01c5

chore: adding generated to files in package.json

0b7dc81

chore: adding more info about protoc

11c5a56

chore: replacing google-protobuf with protobufjs

bfeaec7

chore: merge branch 'master' into collector_proto

7c450b6

chore: cleanup

18b0745

chore: updating proto

5a0cbc5

chore: updating proto

84afac3

chore: merge with master

8fb0274

chore: merge collector proto

1b1a9ba

chore: merge branch 'collector_proto' into proto_update

7448693

chore: merge branch 'master' into proto_update

c321638

chore: fixes for tests

ca84624

chore: merge branch 'master' into proto_update

ef6020f

chore: fixes after merge

90e995f

chore: fixes after merge

a61a261

obecny requested review from dyladan, legendecas, markwolff, mayurkale22, mwear, naseemkullah, OlivierAlbertini and vmarchaud as code owners July 22, 2020 18:15

obecny self-assigned this Jul 22, 2020

obecny added 2 commits July 22, 2020 20:16

chore: cleanup

48740ea

Merge branch 'proto_update' of github.com:obecny/opentelemetry-js int…

8acc46f

…o proto_update

obecny added 3 commits July 22, 2020 20:28

chore: readme

f2fa357

Merge branch 'master' into proto_update

b007d88

chore: adding unit test for mixed values with array and maps

21904d6

dyladan approved these changes Jul 22, 2020

View reviewed changes

obecny changed the title ~~Proto update~~ Proto update to latest to support arrays and maps Jul 22, 2020

obecny added 2 commits July 24, 2020 21:35

chore: aligning default headers with node version as when using beaco…

0ad57e8

…n thye are ignored and when having custom they are ignored too

Merge branch 'master' into proto_update

23af355

mwear approved these changes Jul 27, 2020

View reviewed changes

Merge branch 'master' into proto_update

4a16ac6

mwear mentioned this pull request Jul 27, 2020

chore: 0.10.0 release proposal #1345

Merged

dyladan approved these changes Jul 27, 2020

View reviewed changes

mayurkale22 approved these changes Jul 27, 2020

View reviewed changes

Merge branch 'master' into proto_update

905a5d6

dyladan added the enhancement New feature or request label Jul 27, 2020

Merge branch 'master' into proto_update

4d2e1dd

obecny merged commit 752e8c0 into open-telemetry:master Jul 27, 2020

obecny deleted the proto_update branch July 27, 2020 19:40

jufab mentioned this pull request Jul 28, 2020

Span Attributes are Null open-telemetry/opentelemetry-collector#1385

Closed

jonahrosenblum pushed a commit to jonahrosenblum/opentelemetry-js that referenced this pull request Aug 8, 2020

Proto update to latest to support arrays and maps (open-telemetry#1339)

ab0de32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proto update to latest to support arrays and maps #1339

Proto update to latest to support arrays and maps #1339

obecny commented Jul 22, 2020

codecov bot commented Jul 22, 2020 •

edited

Loading

dyladan left a comment

mwear left a comment

obecny commented Jul 27, 2020

mayurkale22 Jul 27, 2020

obecny Jul 27, 2020 •

edited

Loading

obecny Jul 27, 2020

dyladan Jul 27, 2020

dyladan Jul 27, 2020

mayurkale22 Jul 27, 2020

dyladan commented Jul 27, 2020

Proto update to latest to support arrays and maps #1339

Proto update to latest to support arrays and maps #1339

Conversation

obecny commented Jul 22, 2020

Which problem is this PR solving?

Short description of the changes

codecov bot commented Jul 22, 2020 • edited Loading

Codecov Report

dyladan left a comment

Choose a reason for hiding this comment

mwear left a comment

Choose a reason for hiding this comment

obecny commented Jul 27, 2020

mayurkale22 Jul 27, 2020

Choose a reason for hiding this comment

obecny Jul 27, 2020 • edited Loading

Choose a reason for hiding this comment

obecny Jul 27, 2020

Choose a reason for hiding this comment

dyladan Jul 27, 2020

Choose a reason for hiding this comment

dyladan Jul 27, 2020

Choose a reason for hiding this comment

mayurkale22 Jul 27, 2020

Choose a reason for hiding this comment

dyladan commented Jul 27, 2020

codecov bot commented Jul 22, 2020 •

edited

Loading

obecny Jul 27, 2020 •

edited

Loading