-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
add endpoint metadata struct #43560
add endpoint metadata struct #43560
Conversation
ramaraochavali
commented
Feb 23, 2023
•
edited by istio-policy-bot
Loading
edited by istio-policy-bot
- Ambient
- Configuration Infrastructure
- Docs
- Installation
- Networking
- Performance and Scalability
- Policies and Telemetry
- Security
- Test and Release
- User Experience
- Developer Infrastructure
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
@@ -521,6 +521,39 @@ func (ep *IstioEndpoint) IsDiscoverableFromProxy(p *Proxy) bool { | |||
return ep.DiscoverabilityPolicy.IsDiscoverableFromProxy(ep, p) | |||
} | |||
|
|||
// Metadata returns the endpoint metadata used for telemetry purposes. | |||
func (ep *IstioEndpoint) Metadata() *EndpointMetadata { | |||
return &EndpointMetadata{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think embedding would look cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. That was my original change. But it resulted in too many changes - just worried about regression. I can change like that. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong opinion, just looked like it makes sense here but It's possible it doesn't work elsewhere.
for k, v := range instance.Endpoint.Labels { | ||
labels[k] = v | ||
} | ||
metadata.Labels[model.IstioCanonicalServiceLabelName] = svcLabels[model.IstioCanonicalServiceLabelName] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This causes a concurrent map write from gothreads creating xDS:
fatal error: concurrent map writes
goroutine 1751 [running]:
istio.io/istio/pilot/pkg/networking/core/v1alpha3.(*ClusterBuilder).buildLocalityLbEndpoints(0xc001d2f800, {0x493b8c8, 0x70bc400}, 0xc002bfe2c0, 0xc0040a8480?, 0xc001f57980?)
istio.io/istio/pilot/pkg/networking/core/v1alpha3/cluster_builder.go:578 +0x5c6
istio.io/istio/pilot/pkg/networking/core/v1alpha3.(*ConfigGeneratorImpl).buildOutboundClusters(0xc0006423c0?, 0xc001d2f800, 0x495dce0?, {0xc002f1c450?, 0x18?}, {0xc00309eb80, 0xd, 0x7fa2a444e9f8?})
istio.io/istio/pilot/pkg/networking/core/v1alpha3/cluster.go:257 +0x3f9
istio.io/istio/pilot/pkg/networking/core/v1alpha3.(*ConfigGeneratorImpl).buildClusters(0xc0011b87c0, 0xc0006423c0, 0xc0041ada40, {0xc00309eb80, 0xd, 0x10})
istio.io/istio/pilot/pkg/networking/core/v1alpha3/cluster.go:162 +0x13e
istio.io/istio/pilot/pkg/networking/core/v1alpha3.(*ConfigGeneratorImpl).BuildClusters(0x38e24e0?, 0xc0007259e0?, 0x46a6?)
istio.io/istio/pilot/pkg/networking/core/v1alpha3/cluster.go:88 +0x90
istio.io/istio/pilot/pkg/xds.CdsGenerator.Generate({0x0?}, 0xc0006423c0?, 0xc000c87380?, 0xc0041ada40?)
istio.io/istio/pilot/pkg/xds/cds.go:80 +0xb5
istio.io/istio/pilot/pkg/xds.(*DiscoveryServer).pushXds(0xc0011ac780, 0xc002f8e280, 0xc0006bff80, 0xc0041ada40)
istio.io/istio/pilot/pkg/xds/xdsgen.go:121 +0x2d7
istio.io/istio/pilot/pkg/xds.(*DiscoveryServer).pushConnection(0xc0011ac780, 0xc002f8e280, 0xc002f8e280?)
istio.io/istio/pilot/pkg/xds/ads.go:757 +0x1a5
istio.io/istio/pilot/pkg/xds.(*DiscoveryServer).Stream(0xc0011ac780, {0x495f490, 0xc002853780})
istio.io/istio/pilot/pkg/xds/ads.go:340 +0xa70
istio.io/istio/pilot/pkg/xds.(*DiscoveryServer).StreamAggregatedResources(0xc002cdbb30?, {0x495f490?, 0xc002853780?})
istio.io/istio/pilot/pkg/xds/ads.go:237 +0x25
github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3._AggregatedDiscoveryService_StreamAggregatedResources_Handler({0x407da40?, 0xc0011ac780}, {0x495a510?, 0xc002fd8690})
github.com/envoyproxy/go-control-plane@v0.11.1-0.20230414193417-178e253923d1/envoy/service/discovery/v3/ads.pb.go:298 +0x9f
google.golang.org/grpc.(*Server).processStreamingRPC(0xc000e823c0, {0x4962ca0, 0xc001c551e0}, 0xc0014490e0, 0xc001ce6ab0, 0x6dabd20, 0x0)
google.golang.org/grpc@v1.54.0/server.go:1639 +0x1384
google.golang.org/grpc.(*Server).handleStream(0xc000e823c0, {0x4962ca0, 0xc001c551e0}, 0xc0014490e0, 0x0)
google.golang.org/grpc@v1.54.0/server.go:1726 +0x9f0
google.golang.org/grpc.(*Server).serveStreams.func1.2()
google.golang.org/grpc@v1.54.0/server.go:966 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
google.golang.org/grpc@v1.54.0/server.go:964 +0x28a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which two threads are writing concurrently
Reverting #44470, re-submit after fixing the bug. |