trace: replace zipkin with uptrace

[najeal]

Related to #230

[najeal]

@patrick-ogrady I face one problem when enabling tracing TRACE=true ./scripts/tests.load.sh.
On main branch and uptrace branch test [It] creates blocks fails with error message no transactions.
My first opinion is some transactions are losts.

Except this data is loaded in uptrace as expected.

[patrick-ogrady]

To confirm, this is provided by Uptrace on startup?

[najeal]

Yes it is provided by the running uptrace.
It contains a secret to communicate with uptrace, is it safe to use it in this config?

[patrick-ogrady]

We can set the default value (like I did in my patch).

[patrick-ogrady]

@patrick-ogrady I face one problem when enabling tracing TRACE=true ./scripts/tests.load.sh. On main branch and uptrace branch test [It] creates blocks fails with error message no transactions. My first opinion is some transactions are losts.

Except this data is loaded in uptrace as expected.

Hmmm. Let me look rq.

[patrick-ogrady]

It worked for me:

I just had to do this diff (run go mod tidy and add a default DSN):
patch.txt

[patrick-ogrady]

The next iteration here will also be adding the prometheus metrics ingestion to this tool ❤️

[najeal]

@patrick-ogrady the data is loaded in your uptrace but you confirm that the load tests are successul with trace enabled and uptrace running?

I will add prometheus in the same PR.

[patrick-ogrady]

@patrick-ogrady the data is loaded in your uptrace but you confirm that the load tests are successul with trace enabled and uptrace running?

I will add prometheus in the same PR.

Yep, worked like normal:

What do you see?

[najeal]

@patrick-ogrady I see this:

then :

[patrick-ogrady]

🤔 definitely seems like a new bug. This would happen if the calculation for how many times to call build block (based on the fee and any balance is wrong):

hypersdk/examples/morpheusvm/tests/load/load_test.go

Lines 373 to 381 in 1eb7836

	// sending 1 tx to each account
	remainder := uint64(accts)*transferTxUnits + uint64(1_000_000)
	// leave some left over for root
	fundSplit := (genesisBalance - remainder) / uint64(accts)
	gomega.Ω(fundSplit).Should(gomega.Not(gomega.BeZero()))
	requiredBlocks := accts / maxTxsPerBlock
	if accts%maxTxsPerBlock > 0 {
	requiredBlocks++
	}

This is some of the "jankier" logic in the codebase and think it could be cleaned up quite a bit.

[najeal]

But it works on your side 😩 .
I added some logs
We generate 500 000 txs
Looking at the logs each block from 0 to 109 is created with 4090 txs
the 110th has only 231 txs.
We should have created 122 blocks.

4090 * 110 + 231 = 450 131 txs

50 000 tx have been lost. Is it possible they got removed due to some logic?

[najeal]

@patrick-ogrady I wanted to generate prometheus config to export metrics from prometheus into Uptrace using remote_write. But this method only work for their saas currently.
If we want to use a local Uptrace we need to use an OpenTelemetry Collector.

So ready to be reviewed

[patrick-ogrady]

I'm wondering why we import uptrace at all instead of just passing a header into the sdktrace options below like AvalancheGo: ava-labs/avalanchego#1727

The appeal there is that we can just use the stock OpenTelemetry?

[patrick-ogrady]

In the process, we should just migrate to use this package (we don't need a separate trace package anymore because uptrace can use opentelemtry directly ❤️ ): https://github.com/ava-labs/avalanchego/blob/master/trace/tracer.go

[najeal]

trace package has been updated. If you want to use trace package from avalanchego we need to update avalanchego to pass AppName and Version from the Config. Then we can use it from hypersdk

https://github.com/ava-labs/avalanchego/blob/e70a17d9d988b5067f3ef5c4a057f15ae1271ac4/trace/tracer.go#L73C2-L73C2

https://github.com/ava-labs/avalanchego/blob/e70a17d9d988b5067f3ef5c4a057f15ae1271ac4/trace/tracer.go#L74

[patrick-ogrady]

I think that would be a good change. Can you open an issue for that? We can probably merge that quickly.

[patrick-ogrady]

I think we should try to do so (use the trace package upstream)

[najeal]

It works 👍

[najeal]

@patrick-ogrady issue has been open avalanchego side and PR (ava-labs/avalanchego#1893)

[github-actions]

This PR has become stale because it has been open for 30 days with no activity. Adding the lifecycle/frozen label will exempt this PR from future lifecycle events..

[patrick-ogrady]

@patrick-ogrady issue has been open avalanchego side and PR (ava-labs/avalanchego#1893)

Bumping this on the platform side

[github-actions]

This PR has become stale because it has been open for 30 days with no activity. Adding the lifecycle/frozen label will exempt this PR from future lifecycle events..

[najeal]

is it still relevant? The PR is old and there is a lot of conflicts.