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.

Sign up for GitHub

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

Jump to bottom

data race/concurrency crash #11

Closed
ldemailly opened this issue Jan 2, 2023 · 0 comments
Closed

data race/concurrency crash #11

ldemailly opened this issue Jan 2, 2023 · 0 comments

Comments

@ldemailly
Copy link
Member 

fortio server &
make race-test
# Also crashes without -race
OTEL_SERVICE_NAME=race-test go run -race . load -n 40 -c 2 -qps -1 "http://localhost:8080/?delay=200ms"
Fortio 1.39.0-pre12 running at -1 queries per second, 10->10 procs, for 40 calls: http://localhost:8080/?delay=200ms
15:43:39 I fortio_with_otel.go:143> Fortio OTEL variant dev - export pipeline setup successfully
15:43:39 I httprunner.go:103> Starting http test for http://localhost:8080/?delay=200ms with 2 threads at -1.0 qps and parallel warmup
Starting at max qps with 2 thread(s) [gomax 10] for exactly 40 calls (20 per thread + 0) with access logger otel
==================
WARNING: DATA RACE
Write at 0x00c000112b40 by goroutine 26:
  runtime.mapaccess2_faststr()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/runtime/map_faststr.go:108 +0x43c
  net/textproto.MIMEHeader.Set()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/textproto/header.go:22 +0xa8
  net/http.Header.Set()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/header.go:40 +0x38
  go.opentelemetry.io/otel/propagation.HeaderCarrier.Set()
      /Users/ldemailly/go/pkg/mod/go.opentelemetry.io/otel@v1.11.2/propagation/propagation.go:79 +0x34
  go.opentelemetry.io/contrib/propagators/b3.propagator.Inject()
      /Users/ldemailly/go/pkg/mod/go.opentelemetry.io/contrib/propagators/b3@v1.12.0/b3_propagator.go:116 +0x550
  go.opentelemetry.io/contrib/propagators/b3.(*propagator).Inject()
      <autogenerated>:1 +0x60
  go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp.(*Transport).RoundTrip()
      /Users/ldemailly/go/pkg/mod/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp@v0.37.0/transport.go:114 +0x628
  net/http.send()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/client.go:251 +0x6cc
  net/http.(*Client).send()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/client.go:175 +0x100
  net/http.(*Client).do()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/client.go:715 +0xbf0
  net/http.(*Client).Do()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/client.go:581 +0x5f4
  fortio.org/fortio/fhttp.(*Client).Fetch()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/fhttp/http_client.go:452 +0x5dc
  fortio.org/fortio/fhttp.(*HTTPRunnerResults).Run()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/fhttp/httprunner.go:62 +0xa0
  fortio.org/fortio/periodic.runOne()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/periodic/periodic.go:768 +0x500
  fortio.org/fortio/periodic.(*periodicRunner).Run.func1()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/periodic/periodic.go:549 +0x94
  fortio.org/fortio/periodic.(*periodicRunner).Run.func2()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/periodic/periodic.go:551 +0x6c

Previous read at 0x00c000112b40 by goroutine 25:
  runtime.evacuate_fast64()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/runtime/map_fast64.go:376 +0x3fc
  net/textproto.MIMEHeader.Get()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/textproto/header.go:34 +0x54
  net/http.Header.Get()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/header.go:50 +0x54
  net/http.(*Request).UserAgent()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/request.go:405 +0x30
  go.opentelemetry.io/otel/semconv/internal.(*SemanticConventions).httpCommonAttributesFromHTTPRequest()
      /Users/ldemailly/go/pkg/mod/go.opentelemetry.io/otel@v1.11.2/semconv/internal/http.go:165 +0x58
  go.opentelemetry.io/otel/semconv/internal.(*SemanticConventions).HTTPClientAttributesFromHTTPRequest()
      /Users/ldemailly/go/pkg/mod/go.opentelemetry.io/otel@v1.11.2/semconv/internal/http.go:160 +0x224
  go.opentelemetry.io/otel/semconv/v1%2e12%2e0.HTTPClientAttributesFromHTTPRequest()
      /Users/ldemailly/go/pkg/mod/go.opentelemetry.io/otel@v1.11.2/semconv/v1.12.0/http.go:79 +0x5b0
  go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp.(*Transport).RoundTrip()
      /Users/ldemailly/go/pkg/mod/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp@v0.37.0/transport.go:113 +0x478
  net/http.send()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/client.go:251 +0x6cc
  net/http.(*Client).send()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/client.go:175 +0x100
  net/http.(*Client).do()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/client.go:715 +0xbf0
  net/http.(*Client).Do()
      /opt/homebrew/Cellar/go/1.19.4/libexec/src/net/http/client.go:581 +0x5f4
  fortio.org/fortio/fhttp.(*Client).Fetch()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/fhttp/http_client.go:452 +0x5dc
  fortio.org/fortio/fhttp.(*HTTPRunnerResults).Run()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/fhttp/httprunner.go:62 +0xa0
  fortio.org/fortio/periodic.runOne()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/periodic/periodic.go:768 +0x500
  fortio.org/fortio/periodic.(*periodicRunner).Run.func1()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/periodic/periodic.go:549 +0x94
  fortio.org/fortio/periodic.(*periodicRunner).Run.func2()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/periodic/periodic.go:551 +0x6c

Goroutine 26 (running) created at:
  fortio.org/fortio/periodic.(*periodicRunner).Run()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/periodic/periodic.go:548 +0x135c
  fortio.org/fortio/fhttp.RunHTTPTest()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/fhttp/httprunner.go:186 +0xef4
  fortio.org/fortio/cli.fortioLoad()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/cli/fortio_main.go:479 +0x1080
  fortio.org/fortio/cli.FortioMain()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/cli/fortio_main.go:245 +0x8d8
  main.main()
      /Users/ldemailly/dev/fortio.org/fortiotel/fortio_with_otel.go:159 +0xa0

Goroutine 25 (running) created at:
  fortio.org/fortio/periodic.(*periodicRunner).Run()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/periodic/periodic.go:548 +0x135c
  fortio.org/fortio/fhttp.RunHTTPTest()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/fhttp/httprunner.go:186 +0xef4
  fortio.org/fortio/cli.fortioLoad()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/cli/fortio_main.go:479 +0x1080
  fortio.org/fortio/cli.FortioMain()
      /Users/ldemailly/go/pkg/mod/fortio.org/fortio@v1.39.0-pre12/cli/fortio_main.go:245 +0x8d8
  main.main()
      /Users/ldemailly/dev/fortio.org/fortiotel/fortio_with_otel.go:159 +0xa0
==================
[...]
@ldemailly ldemailly mentioned this issue Jan 2, 2023
Merged
5 tasks
ldemailly added a commit to fortio/fortio that referenced this issue Jan 3, 2023
@ldemailly
fix fortio/fortiotel#11 make sure each request/client/thread has thei…
4b9d3d8 
…r own headers
ldemailly added a commit to fortio/fortio that referenced this issue Jan 14, 2023
@ldemailly
adding optional ClientTrace and Context to jrpc and fhttp; extend acc…
cc145c4 
…ess log api to be usable for tracing; add optional Transport chaining for otelhttp (#661)

* adding optional ClientTrace and Context to jrpc

* also allow ClientTrace in fhttp runner (std client) too

* move the main functions/code into a package so it can be reused with additional hooks in other binaries like experimental OTEL fortio

* added optional hook to mutate options etc in new FortioMain. also show value for catchup/nocatchup. fix serialization error on ClientTrace when not nil

* add a before (Start()) hook to the access logger so it can be used to start otel traces/spans

* pass context around, use ThreadID as a type

* client trace must use ctx with outer span so creation has to be delayed, so we need to make the option a factory method instead of the trace ptr directly

* set uniforn and nocatchup as default in the web UI

* also call the hook for curl case

* make usage changeable for FortioMain

* show container version in full version for library/embed case

* show long version in footer of ui (to identify fortiotel)

* bite the bullet and also allow Transport to be changed through options (for fortiotel)

* fix fortio/fortiotel#11 make sure each request/client/thread has their own headers (pre-existing bug in a way)

* Adding link to otel variant in readme

* adding ChangeFlagsDefault slightly modified/improved from fortiotel
ldemailly added a commit to fortio/log that referenced this issue Feb 11, 2023
@ldemailly
adding optional ClientTrace and Context to jrpc and fhttp; extend acc…
c775137 
…ess log api to be usable for tracing; add optional Transport chaining for otelhttp (#661)

* adding optional ClientTrace and Context to jrpc

* also allow ClientTrace in fhttp runner (std client) too

* move the main functions/code into a package so it can be reused with additional hooks in other binaries like experimental OTEL fortio

* added optional hook to mutate options etc in new FortioMain. also show value for catchup/nocatchup. fix serialization error on ClientTrace when not nil

* add a before (Start()) hook to the access logger so it can be used to start otel traces/spans

* pass context around, use ThreadID as a type

* client trace must use ctx with outer span so creation has to be delayed, so we need to make the option a factory method instead of the trace ptr directly

* set uniforn and nocatchup as default in the web UI

* also call the hook for curl case

* make usage changeable for FortioMain

* show container version in full version for library/embed case

* show long version in footer of ui (to identify fortiotel)

* bite the bullet and also allow Transport to be changed through options (for fortiotel)

* fix fortio/fortiotel#11 make sure each request/client/thread has their own headers (pre-existing bug in a way)

* Adding link to otel variant in readme

* adding ChangeFlagsDefault slightly modified/improved from fortiotel
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ldemailly