[BEAM-1542] Specifies a User Agent in Spanner Client #3574
Conversation
|
retest this please |
|
|
||
| /** | ||
| * Abstract {@link DoFn} that manages {@link Spanner} lifecycle. Use {@link | ||
| * AbstractSpannerFn#databaseClient} to access the Cloud Spanner database client. | ||
| */ | ||
| abstract class AbstractSpannerFn<InputT, OutputT> extends DoFn<InputT, OutputT> { | ||
| private static final String USER_AGENT_PREFIX = "apache.beam.java"; |
There was a problem hiding this comment.
Add a comment: any reason for this particular format? Is this similar to the format of other Spanner user agents?
There was a problem hiding this comment.
According to https://tools.ietf.org/html/rfc7231#page-46
format should be like User-Agent: CERN-LineMode/2.15 libwww/2.17b3
We just want to make sure we can track the usage of the library.
| builder.setServiceFactory(spannerConfig.getServiceFactory()); | ||
| } | ||
| ReleaseInfo releaseInfo = ReleaseInfo.getReleaseInfo(); | ||
| builder.setUserAgentPrefix(USER_AGENT_PREFIX + "." + releaseInfo.getVersion()); |
There was a problem hiding this comment.
Maybe use ReleaseInfo.getName() too? I don't know if ReleaseInfo.getProperties() is useful or not.
There was a problem hiding this comment.
Version should be enough, the name appears to be static.
There was a problem hiding this comment.
I would not be surprised if at some point ReleaseInfo.getName() may return a non-hardcoded value; e.g. if somebody is using Beam via https://github.com/spotify/scio/, I think it'd be nice if ReleaseInfo said that this is scio, even though currently it doesn't say that. There doesn't seem to any downside to using ReleaseInfo.getName() instead of hardcoding "Apache_Beam_Java" yourself, is there?
R: @jkff