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

Fix default docker_observer endpoint on Windows #34358

Merged

Conversation

pjanotti
Copy link
Contributor

@pjanotti pjanotti commented Jul 31, 2024

Description:
The default endpoint for the docker_observer is incorrect on Windows: it should be npipe:////./pipe/docker_engine on Windows. Without this change the observer fails on its default configuration on Windows.

Updated extension/observer/dockerobserver and internal/docker (including respective tests) to have a per OS default value for endpoint that allows the observer to work with default config on Windows.

Link to tracking Issue: N/A

Testing:
Manual tests using the following config:

extensions:
  docker_observer:

receivers:
  receiver_creator:
    watch_observers: [docker_observer]
    receivers:
      nginx:
        rule: type == "container" and name matches "nginx" and port == 80
        config:
          endpoint: '`endpoint`/status'
          collection_interval: 10s

exporters:
  debug:
    verbosity: detailed

service:
  extensions: [docker_observer]
  pipelines:
    logs:
      receivers: [receiver_creator]
      processors: []
      exporters: [debug]

Documentation:

  • Updated readme files
  • chloggen (pending PR number)

Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

Does this need a changelog?

@pjanotti
Copy link
Contributor Author

Yes, I was waiting the PR number to create one.

@pjanotti
Copy link
Contributor Author

also more go mod tidy ...

…janotti/opentelemetry-service-contrib into fix-default-discovery-crash-on-windows
@pjanotti
Copy link
Contributor Author

Unrelated windows-unit-test failures, see #34252

=== FAIL: compression/zstd TestMRUReset (0.00s)
    mru_test.go:83: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/internal/otelarrow/compression/zstd/mru_test.go:83
        	Error:      	Not equal: 
make[2]: *** [../../Makefile.Common:126: test] Error 1
        	            	expected: 0
        	            	actual  : 1
        	Test:       	TestMRUReset

make[1]: *** [Makefile:179: internal/otelarrow] Error 2
=== FAIL: compression/zstd TestMRUReset (re-run 1) (0.00s)
make[1]: *** Waiting for unfinished jobs....
    mru_test.go:83: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/internal/otelarrow/compression/zstd/mru_test.go:83
        	Error:      	Not equal: 
        	            	expected: 0
        	            	actual  : 1
        	Test:       	TestMRUReset

@pjanotti
Copy link
Contributor Author

@pjanotti
Copy link
Contributor Author

pjanotti commented Jul 31, 2024

Not sure about this one https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/10187830131/job/28182730990?pr=34358#step:6:21

/usr/bin/sh: line 1: D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum: Device or resource busy
make[2]: *** [../../Makefile.Common:126: test] Error 126
make[1]: *** [Makefile:179: processor/cumulativetodeltaprocessor] Error 2
make[1]: *** Waiting for unfinished jobs....
D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum --rerun-fails=1 --packages="./..." -- -race -timeout 600s -parallel 4 --tags=""

@pjanotti
Copy link
Contributor Author

Also hitting #34297

@pjanotti
Copy link
Contributor Author

pjanotti commented Aug 2, 2024

Similar to the above, but, a different component and symptom https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/10207176086/job/28241472190?pr=34358#step:6:21

/usr/bin/sh: line 1: D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum: cannot execute: required file not found
make[2]: *** [../../Makefile.Common:126: test] Error 127
make[1]: *** [Makefile:179: pkg/batchperresourceattr] Error 2
make[1]: *** Waiting for unfinished jobs....
D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum --rerun-fails=1 --packages="./..." -- -race -timeout 600s -parallel 4 --tags=""

@pjanotti
Copy link
Contributor Author

pjanotti commented Aug 7, 2024

/label os:windows

@crobert-1
Copy link
Member

/label os:windows

Just an FYI, I don't believe adding labels via comments works on PRs. (Source)

@crobert-1 crobert-1 added the ready to merge Code review completed; ready to merge by maintainers label Aug 7, 2024
@pjanotti
Copy link
Contributor Author

pjanotti commented Aug 7, 2024

@crobert-1 yes, you're right that didn't work here - it did work earlier on an issue 👍🏼

@pjanotti
Copy link
Contributor Author

pjanotti commented Aug 7, 2024

Another hit on gotestsum https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/10287492291/job/28470657160?pr=34358#step:6:20

D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum --rerun-fails=1 --packages="./..." -- -race -timeout 600s -parallel 4 --tags=""
/usr/bin/sh: line 1: D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum: Device or resource busy
make[2]: *** [../../Makefile.Common:126: test] Error 126
make[1]: *** [Makefile:179: processor/probabilisticsamplerprocessor] Error 2
make[1]: *** Waiting for unfinished jobs....
D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/.tools/gotestsum --rerun-fails=1 --packages="./..." -- -race -timeout 600s -parallel 4 --tags=""

@dmitryax dmitryax merged commit 4d97bc2 into open-telemetry:main Aug 7, 2024
184 of 216 checks passed
@github-actions github-actions bot added this to the next release milestone Aug 7, 2024
f7o pushed a commit to f7o/opentelemetry-collector-contrib that referenced this pull request Sep 12, 2024
**Description:**
The default endpoint for the `docker_observer` is incorrect on Windows:
it should be `npipe:////./pipe/docker_engine` on Windows. Without this
change the observer fails on its default configuration on Windows.

Updated `extension/observer/dockerobserver` and `internal/docker`
(including respective tests) to have a per OS default value for endpoint
that allows the observer to work with default config on Windows.

**Link to tracking Issue:** N/A

**Testing:**
Manual tests using the following config:
```yaml
extensions:
  docker_observer:

receivers:
  receiver_creator:
    watch_observers: [docker_observer]
    receivers:
      nginx:
        rule: type == "container" and name matches "nginx" and port == 80
        config:
          endpoint: '`endpoint`/status'
          collection_interval: 10s

exporters:
  debug:
    verbosity: detailed

service:
  extensions: [docker_observer]
  pipelines:
    logs:
      receivers: [receiver_creator]
      processors: []
      exporters: [debug]
```

**Documentation:**
- [X] Updated readme files
- [X] chloggen (~~pending PR number~~)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension/observer internal/docker os:windows ready to merge Code review completed; ready to merge by maintainers receiver/dockerstats Run Windows Enable running windows test on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants