Skip to content

Make metric / log exporter closeable and improve behavior of some tests#4123

Merged
jkwatson merged 1 commit intoopen-telemetry:mainfrom
anuraaga:better-behaved-tests
Jan 26, 2022
Merged

Make metric / log exporter closeable and improve behavior of some tests#4123
jkwatson merged 1 commit intoopen-telemetry:mainfrom
anuraaga:better-behaved-tests

Conversation

@anuraaga
Copy link
Copy Markdown
Contributor

Enabling output streams on tests shows some places that are not closing gRPC channels, returning null from FooExporter (which fails when putting into the SimpleSpanProcessor's pending task map), and incorrect kotlin coroutines usage.

delay(10)
assertThat(Context.current().get(ANIMAL)).isEqualTo("cat")
for (i in 0 until 100) {
GlobalScope.launch {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why I used GlobalScope here but it was wrong, it starts a job in a new context meaning

  1. The ANIMAL is null, not cat
  2. the result is not collected into runBlocking (so assertion failure was getting ignored)

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 26, 2022

Codecov Report

Merging #4123 (cdd7828) into main (0a2f400) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #4123      +/-   ##
============================================
- Coverage     90.21%   90.20%   -0.02%     
- Complexity     4506     4507       +1     
============================================
  Files           532      532              
  Lines         13824    13829       +5     
  Branches       1322     1322              
============================================
+ Hits          12472    12474       +2     
- Misses          919      922       +3     
  Partials        433      433              
Impacted Files Coverage Δ
.../io/opentelemetry/sdk/logs/export/LogExporter.java 91.66% <100.00%> (+1.66%) ⬆️
...entelemetry/sdk/metrics/export/MetricExporter.java 66.66% <100.00%> (+66.66%) ⬆️
...entelemetry/exporter/jaeger/PostSpansResponse.java 0.00% <0.00%> (-100.00%) ⬇️
...exporter/jaeger/MarshalerCollectorServiceGrpc.java 85.71% <0.00%> (-4.77%) ⬇️
...er/otlp/internal/traces/TraceRequestMarshaler.java
...etry/exporter/otlp/internal/okhttp/OkHttpUtil.java
...tlp/internal/metrics/NumberDataPointMarshaler.java
...metry/exporter/otlp/internal/CodedInputStream.java
...etrics/ExponentialHistogramDataPointMarshaler.java
.../otlp/internal/grpc/OkHttpGrpcExporterBuilder.java
... and 126 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 0a2f400...cdd7828. Read the comment docs.

@anuraaga
Copy link
Copy Markdown
Contributor Author

/easycla

@jkwatson jkwatson merged commit 3cabcc7 into open-telemetry:main Jan 26, 2022
anuraaga added a commit to anuraaga/opentelemetry-java that referenced this pull request Jan 28, 2022
anuraaga added a commit that referenced this pull request Feb 1, 2022
#4130)

* Improve behavior of some tests (#4123)

* Optimize

* Fix logging.properties

* Clean
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants