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

integrate beholder client #14110

Merged
merged 1 commit into from
Sep 11, 2024
Merged

integrate beholder client #14110

merged 1 commit into from
Sep 11, 2024

Conversation

jmank88
Copy link
Contributor

@jmank88 jmank88 commented Aug 13, 2024

patrickhuie19
patrickhuie19 previously approved these changes Aug 14, 2024
core/config/docs/core.toml Outdated Show resolved Hide resolved
core/cmd/shell.go Outdated Show resolved Hide resolved
@jmank88 jmank88 force-pushed the beholder-client branch 2 times, most recently from f9c428f to 1b9b490 Compare August 22, 2024 23:50
@jmank88 jmank88 force-pushed the beholder-client branch 4 times, most recently from f248689 to 21244c5 Compare August 27, 2024 12:24
@jmank88 jmank88 force-pushed the beholder-client branch 7 times, most recently from 17c07c2 to fad1ee9 Compare September 5, 2024 17:20
@jmank88 jmank88 requested a review from a team as a code owner September 9, 2024 17:50
@jmank88 jmank88 force-pushed the beholder-client branch 8 times, most recently from d6dc83e to cbfe80c Compare September 10, 2024 11:56
core/cmd/shell.go Outdated Show resolved Hide resolved
pkcll
pkcll previously approved these changes Sep 10, 2024
err = multierr.Append(err, configutils.ErrInvalid{Name: "RootDir", Value: true, Msg: fmt.Sprintf("Failed to expand RootDir. Please use an explicit path: %s", verr)})
}

if (*c.OCR.Enabled || *c.OCR2.Enabled) && !*c.P2P.V2.Enabled {
err = multierr.Append(err, configutils.ErrInvalid{Name: "P2P.V2.Enabled", Value: false, Msg: "P2P required for OCR or OCR2. Please enable P2P or disable OCR/OCR2."})
}

if *c.Tracing.Enabled && *c.Telemetry.Enabled {
Copy link
Contributor

Choose a reason for hiding this comment

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

If both are enabled will there be a clash over how the globally registered tracing provider is set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, TraceSpanExporter is used to register a second exporter.

I did notice an issue with the attributes merge though. Fix incoming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jmank88 jmank88 added this pull request to the merge queue Sep 11, 2024
Merged via the queue into develop with commit 8454f46 Sep 11, 2024
153 checks passed
@jmank88 jmank88 deleted the beholder-client branch September 11, 2024 21:05
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.

3 participants