Skip to content

[examples/demo] fix demo could not stat up#16169

Merged
mx-psi merged 2 commits into
open-telemetry:mainfrom
JaredTan95:fix_demo_statrup
Nov 16, 2022
Merged

[examples/demo] fix demo could not stat up#16169
mx-psi merged 2 commits into
open-telemetry:mainfrom
JaredTan95:fix_demo_statrup

Conversation

@JaredTan95

Copy link
Copy Markdown
Member

Signed-off-by: Jared Tan jian.tan@daocloud.io

Description:

fix #16090

Link to tracking Issue:

Testing:

Documentation:

Signed-off-by: Jared Tan <jian.tan@daocloud.io>
@JaredTan95

JaredTan95 commented Nov 8, 2022

Copy link
Copy Markdown
Member Author

/skip-changelog

@PassionateDeveloper86

Copy link
Copy Markdown

This sadly doesnt fix the bug on my machine (W11, Docker for Windows), same error when I apply your fix manuelly and rebuild the images

@JaredTan95

JaredTan95 commented Nov 8, 2022

Copy link
Copy Markdown
Member Author

This sadly doesnt fix the bug on my machine (W11, Docker for Windows), same error when I apply your fix manuelly and rebuild the images

It doesn't make sense, I tested in my local dev. I will test it again.

@JaredTan95

Copy link
Copy Markdown
Member Author

@PassionateDeveloper86 I update dockerfile, could you test it with apple M1 ?

@saurabhdes

Copy link
Copy Markdown

Thankyou @JaredTan95 for the PR. I will check it on my Mac M1 Pro later today and post the result here.

@saurabhdes

saurabhdes commented Nov 8, 2022

Copy link
Copy Markdown

@JaredTan95 , I checked out your fork and branch - good news is it's now working on my Mac M1 Pro, but with below caveats:

  1. 1st time i ran docker compose up -d , demo-server started successfully but demo-client failed with error 2022/11/08 17:53:35 Failed to create the collector trace exporter: context deadline exceeded
  2. The i ran docker compose up -d again, after that demo-client started sucessfully.

I can reproduce this consistently, by running

  1. docker-compose down
  2. docker-compose up -d , notice error in demo-client
  3. docker-compose up -d, now demo-client runs successfully

Attached some screenshots - the 1st and 2nd show the error, the 3rd and 4th show the resolution after running Step 3 above.
Screenshot 2022-11-08 at 9 47 19 AM

Screenshot 2022-11-08 at 9 50 39 AM

Screenshot 2022-11-08 at 9 47 46 AM

Screenshot 2022-11-08 at 9 48 02 AM

@JaredTan95

Copy link
Copy Markdown
Member Author

yes, it's another issue,because of otel collector not ready before demo-client start up.

@saurabhdes

Copy link
Copy Markdown

Yes, i agree it's a separate issue. I found a workaround which is to have restart: always in demo-client's docker compose specification, but we may need some time to figure out a better alternative.

@JaredTan95

JaredTan95 commented Nov 9, 2022

Copy link
Copy Markdown
Member Author

Yes, i agree it's a separate issue. I found a workaround which is to have restart: always in demo-client's docker compose specification, but we may need some time to figure out a better alternative.

restart: always make sense. i will update it.

another solution, add otel healthcheck for demo-client in docker-compose.

@JaredTan95 JaredTan95 changed the title [demo/dockerfile] fix demo could not stat up [examples/demo] fix demo could not stat up Nov 9, 2022
@saurabhdes

saurabhdes commented Nov 10, 2022

Copy link
Copy Markdown

thanks @JaredTan95 , checked out latest commit on my M1 Pro and all containers are starting correctly. PR looks good from my side .

@JaredTan95

Copy link
Copy Markdown
Member Author

thanks @JaredTan95 , checked out latest commit on my M1 Pro and all containers are starting correctly. PR looks good from my side .

thx for your testing. 👍

@mx-psi mx-psi added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Nov 15, 2022
Comment thread examples/demo/client/Dockerfile Outdated
Signed-off-by: Jared Tan <jian.tan@daocloud.io>
@JaredTan95

Copy link
Copy Markdown
Member Author

@mx-psi pls help review this :-P

@mx-psi mx-psi merged commit d9b5dcc into open-telemetry:main Nov 16, 2022
@JaredTan95 JaredTan95 deleted the fix_demo_statrup branch November 16, 2022 09:44
JaredTan95 added a commit to openinsight-proj/opentelemetry-collector-contrib that referenced this pull request Nov 21, 2022
* fix demo could not stat up

Signed-off-by: Jared Tan <jian.tan@daocloud.io>

* remove unnecessary GOOS=linux

Signed-off-by: Jared Tan <jian.tan@daocloud.io>

Signed-off-by: Jared Tan <jian.tan@daocloud.io>
shalper2 pushed a commit to shalper2/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2022
* fix demo could not stat up

Signed-off-by: Jared Tan <jian.tan@daocloud.io>

* remove unnecessary GOOS=linux

Signed-off-by: Jared Tan <jian.tan@daocloud.io>

Signed-off-by: Jared Tan <jian.tan@daocloud.io>
@plantfansam plantfansam mentioned this pull request Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples/demo Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Demo example not working on Mac M1 Pro

6 participants