Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose service's status channel #1042

Merged
merged 3 commits into from
May 29, 2020

Conversation

pavolloffay
Copy link
Member

Signed-off-by: Pavol Loffay ploffay@redhat.com

Description:

Resolves #1033

Link to tracking Issue:

Testing: < Describe what testing was performed and which tests were added.>

Documentation: < Describe the documentation added.>

@codecov
Copy link

codecov bot commented May 28, 2020

Codecov Report

Merging #1042 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1042      +/-   ##
==========================================
+ Coverage   86.33%   86.35%   +0.02%     
==========================================
  Files         199      199              
  Lines       14072    14078       +6     
==========================================
+ Hits        12149    12157       +8     
+ Misses       1461     1460       -1     
+ Partials      462      461       -1     
Impacted Files Coverage Δ
service/service.go 50.52% <100.00%> (+1.06%) ⬆️
translator/internaldata/resource_to_oc.go 76.47% <0.00%> (+2.94%) ⬆️

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 034d92d...f9989b9. Read the comment docs.

@tigrannajaryan
Copy link
Member

@pavolloffay can you clarify how this is intended to be used? If you can please show the code that will use it.

@pavolloffay
Copy link
Member Author

We would like to use this in Jaeger all-in-one component that combines OTEL collector and Jaeger query in a single executable.

Since the OTEL service listens on multiple terminations signals: sigterm and asyncErrorChannel we need an aggregated way to notify our service that shutdown is initiated and when it's done.

        // query runs in a separate goroutine. 
	queryServer, tracerCloser := startQuery(v, logger, storageFactory, svc.ReportFatalError)
	for state := range svc.GetStateChannel() {
		switch state {
		case service.ClosingDown:
			queryServer.Close()
			tracerCloser.Close()
		case service.Closed:
			return
		}
	}

@tigrannajaryan tigrannajaryan self-assigned this May 28, 2020
@pavolloffay
Copy link
Member Author

@tigrannajaryan I will fix the coverage once we agree on the implementation.

@tigrannajaryan
Copy link
Member

@pavolloffay approach LGTM.

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@pavolloffay
Copy link
Member Author

It should be ready for review.

@tigrannajaryan tigrannajaryan merged commit 366363c into open-telemetry:master May 29, 2020
wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this pull request Jul 13, 2020
Resolves open-telemetry#1033

We would like to use this in Jaeger `all-in-one` component that combines OTEL collector and Jaeger query in a single executable.

Since the OTEL service listens on multiple terminations signals: `sigterm` and `asyncErrorChannel` we need an aggregated way to notify our service that shutdown is initiated and when it's done.

```go
        // query runs in a separate goroutine. 
	queryServer, tracerCloser := startQuery(v, logger, storageFactory, svc.ReportFatalError)
	for state := range svc.GetStateChannel() {
		switch state {
		case service.ClosingDown:
			queryServer.Close()
			tracerCloser.Close()
		case service.Closed:
			return
		}
	}
```
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021
* Renaming OTEL_RESOURCE_LABELS env var

As per the specification here open-telemetry/opentelemetry-specification#758

* update changelog
swiatekm pushed a commit to swiatekm/opentelemetry-collector that referenced this pull request Oct 9, 2024
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.

Subscribe to shutdown event
3 participants