-
Notifications
You must be signed in to change notification settings - Fork 131
Specifying .Destructure.ByTransforming<T>() in the config #106
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
Comments
I think it would have to be new config syntax specifically customized for this purpose. Regardless of the sources used to build the final I think it would require a whole new top level section (like I doubt there would be a good way to capture the "where" condition for |
After thinking about this a lot more, I'm pretty sure I can populate @nblumhardt Nick, opinions? For Serilog in general, I'm not completely sure how broadly useful |
I think this is getting beyond what we want to attempt using config. (Definitely in the "hold my beer" category :-D ) Supporting |
Are you referring to I do feel like I see a lot of |
No, the config system works based on extension methods (for better or worse :-)), so |
Sounds interesting, but still seems like more than we'd want to take on, here. Perhaps a middle ground would be to create an extension library to support:
This would allow integration into config using the standard mechanism. We already do this for Serilog.Filters.Expressions - check out the README/code there to see how it all fits together (pretty neatly!) |
Where is that extension method? |
Interesting. I was actually considering a Roslyn-based... ah just saw your other reply -- yes, an extension library is what I was sort of working my way towards. But I'm not quite clear how it would integrate with configuration without I'll have to study the expression project more. This level of flexibility is very intriguing to me. |
@nblumhardt This afternoon I whipped up a quick-and-dirty extension for https://github.com/MV10/serilog-extensions-roslyn There are two things I don't like. There are a few seconds of startup overhead to grab all the loaded assemblies and namespaces. It probably wouldn't be a big deal in a long-running system like a web app, and for performance-sensitive scenarios, I have ideas to optimize this by adding overloads as well as setup methods that would allow users to provide lists of Types to reference (I think I could build what I need from there). I also don't like using The Roslyn part is remarkably simple (which is amazing to me). Edit - The other thing I'll try tomorrow is populating the filter expression with this Config package, but it's just a |
@MV10 that's very slick :-) If you decide to keep pushing it towards "production" usability, we can link it from the "community projects" wiki page: https://github.com/serilog/serilog/wiki/Community-Projects |
This is really awesome @MV10 I just might end up using that. |
@zippy1981 Plenty of work to do but it should do the trick. @nblumhardt I'm definitely planning to forge ahead with this one as soon as I get that SQL PR wrapped up. It's really only useful in the context of configuration (I can't see any point in writing a lambda in a string for config-by-code), and I'm not sure naming by technique (Roslyn) is better than naming for intent. I'm thinking maybe Serilog.Settings.Delegates instead. If you like, I'd be happy to turn it over to the Serilog project in general, I don't feel any particular need to personally own it in perpetuity (though I'd stay involved under Serilog as well). |
I suppose this also means I'll be adding |
Not sure if this is related and may find some commonality, but I have a repo in here, Serilog.Settings.Code, where I load the config as C# from an arbitrary file (typically a It is based on |
@tsimbalar That's a cool idea. |
Confession time... I was surprised the predicate in Hmm, is this perhaps a way to reference a static switch on the input Even more surprising to me, returning What am I missing here? 😕 Log.Logger = new LoggerConfiguration()
.Destructure.ByTransformingWhere<Account>(
t => false,
a => new { a.id, a.Username, a.AccountType })
.WriteTo.Console().CreateLogger(); Log.Information("Login for {@User}", acct); |
I would still like clarification about https://github.com/MV10/serilog-settings-delegates An example (configured in code since the Serilog Config package doesn't support destructing yet): string transType = "Sample.Account";
string trans = "a => new { a.id, a.Username, a.AccountType }";
string pred = "t => false";
Log.Logger = new LoggerConfiguration()
.Destructure.ByTransformingWhere(pred, transType, trans)
.WriteTo.Console().CreateLogger();
DestructureExamples_GenerateLogs(); One annoying side-effect I'm researching is that it was necessary to resort to private static dynamic EvaluateFuncTValueObject(string transformedType, string transformation)
{
Type TValue = ReflectionHelper.IncludeType(transformedType);
Type funcType = typeof(Func<,>).MakeGenericType(new Type[] { TValue, typeof(object) });
var evalMethod = typeof(CSharpScript).GetMethods()
.FirstOrDefault(m => m.Name.Equals("EvaluateAsync") && m.IsGenericMethod)
.MakeGenericMethod(funcType);
dynamic compiledFunc = evalMethod.InvokeAsync(null, new object[] { transformation, ReflectionHelper.scriptOptions, null, null, null }).GetAwaiter().GetResult();
return compiledFunc;
} In a perfect world, the next-to-last line would be: funcType compiledFunc = evalMethod.InvokeAsync(...) ...but that results in a As a result you get junk like this in the output (the first is with a predicate returning false, the second is with destructuring applied): Finally, I experimented with an optional list of types to reference/import, but it was only a tiny fraction of a second faster than just referencing all loaded assemblies and namespaces. |
@MV10 the |
@tsimbalar That does look related, but now I'm even more confused ... if I'm following that discussion properly, it seems like the only predicate that makes sense would be something like I'd been assuming
Edit: Also, if that's what |
As the creator of I can also confirm it’s a confusing method signature, and I am very open to proposals for a better way. |
@adamchester I'm still trying to understand what purpose it serves, though. Does the non-where version just call the transformation on any type (and, I suppose, fail silently unless the type happens to match)? Ah... looks like I need to digest this serilog/serilog#730. |
The purpose it serves is to control the type matching predicate (“where”). The non-where version always does an EXACT type match, and so you can never match reflection objects like System.Type. If this is confusing, try to destructure something like this:
See if you ever make the destructure configuration “match” this. |
Thanks for the clarification. I think I follow. Maybe the method name could have clarified this somewhat (perhaps I don't think a |
@MV10 what about this? void Main() {
using (var logger = new LoggerConfiguration()
// .Destructure.ByTransforming<dynamic>(d => d.ToPrettyJson())
.Destructure.ByTransformingWhere<dynamic>(predicate: IsDynamic, transformation: dy => dy.ToPrettyJson())
.WriteTo.Console()
.CreateLogger())
{
dynamic MyDynamic = new System.Dynamic.ExpandoObject();
MyDynamic.P1 = "value of property 1";
MyDynamic.P2 = "value of property 2";
MyDynamic.P3 = 3;
logger.Information("test {@Object}", MyDynamic);
// => [13:22:53 INF] test [{"Key": "P1", "Value": "value of property 1", "$type": "KeyValuePair`2"}, {"Key": "P2", "Value": "value of property 2", "$type": "KeyValuePair`2"}, {"Key": "P3", "Value": 3, "$type": "KeyValuePair`2"}]
}
}
static bool IsDynamic(Type t)
{
return t.GetCustomAttribute(typeof(System.Runtime.CompilerServices.DynamicAttribute)) != null;
} |
@adamchester I recall reading that an expando, an anonymous dynamic, and a dynamic wrapping a strong type are very different. Hmm, maybe I don't understand the purpose after all. It doesn't look to me like Whatever I do, I always get the full output, just like your comment:
Going back to your earlier Log.Logger = new LoggerConfiguration()
.Destructure.ByTransformingWhere<Type>(
predicate: t => typeof(Type).IsAssignableFrom(t),
transformation: a => new { a.Namespace })
.WriteTo.Console().CreateLogger();
Log.Information("The type was {@Type}", typeof(String)); |
Ah, this combination works... dynamic as the generic type, but since it does wrap a concrete type (even if it's just Log.Logger = new LoggerConfiguration()
.Destructure.ByTransformingWhere<dynamic>(
predicate: t => typeof(Type).IsAssignableFrom(t),
transformation: n => new { n.Namespace })
.WriteTo.Console().CreateLogger();
Log.Information("The type was {@Type}", typeof(String)); // this will match (will apply transformation)
DestructureExamples_GenerateLogs(); // will not match (no transformation) |
Sorry for the bad example, I should have verified it more before posting. The point I was attempting to make was showing a usage of the method void Main()
{
using (var logger = new LoggerConfiguration()
.Destructure.ByTransformingWhere<dynamic>(
predicate: QuacksLikeAVersion,
transformation: dy => new
{
Version = $"{dy.Major}.{dy.Minor}.{dy.Build}",
OriginalTypeName = dy.GetType().Name as string,
})
.WriteTo.Console(outputTemplate: " * {Message}{NewLine}")
.CreateLogger())
{
logger.Information("Does it quack like a version? {@Object}", new { Build = 1, Major = 1, Minor = 1 });
logger.Information("Does it quack like a version? {@Object}", new Version(1, 2, 3, 4));
logger.Information("Does it quack like a version? {@Object}", DBNull.Value);
logger.Information("Does it quack like a version? {@Object}", new VersionQuacker());
}
}
static bool QuacksLikeAVersion(Type theType)
{
return new[] { "Build", "Major", "Minor" }.All(n => theType.GetProperties().Any(p => p.Name == n));
}
class VersionQuacker
{
public int Major { get; } = 2;
public int Minor { get; } = 3;
public int Build { get; } = 1;
} |
Ten thumbs-up for what I assume is an awesome duck-typing pun. Seriously though, that's crystal-clear and pretty cool. Thank you. One of us should PR that into the Serilog wiki topic on destructuring! Now digging into the arcane guts of generics type-array storage trying to figure out how to trick |
Should I close this issue or keep this open under the assumption that what @MV10 is writing is a prototype for a PR into this repo? |
Sorry, I feel a bit like I hijacked your thread, but personally I'd like to see it remain open. The use-case you asked about is already working. I don't know where this will lead, but I feel enough pieces are already working that it's worth more of my time. Besides, the wife's out of town this weekend, it'll keep me out of the bars. The wrap-up of much of the recent discussion is that I strongly doubt Edit for posterity: It turns out But as I said earlier, |
@MV10 I have not had enough time to Digest the code or comments here that you wrote. I really appreciate what you did. I'm ok with you leaving this open. Just curious as to what the end game is. I asked if something is possible, you wrote a thing to make it possible. I'll probably use that thing. However, this is my first go round with Serilog, and the logging changes I made showed a lot of things to me. |
The end-game is up to Nick. The Filter delegates work with this Configuration repo today, but if he thinks there is enough value in what I'm doing, I'd have to update the Configuration repo to support The other thing I'd like to see is Serilog itself filter out that ugly As for catching up, I wouldn't waste too much time on all my blather (much of it was Adam very patiently helping me wrap my head around the predicate-based transformation method). One way or another (either as part of Serilog or in my own repo) I'll end up documenting it. |
@nblumhardt I've done all I can in terms of exploring this capability. Sink and Audit configs don't have anything that would benefit from this right now. My repo is up to date and has several new examples. Recap:
Arguments for adding the code directly:
Nick, I'm happy to do a proof-of-concept fork of Serilog.Settings.Configuration if you're willing to consider a PR adding this. The real thing would need a little extra Filtering examples using configuration: {
"Serilog": {
"Using": [ "Serilog.Sinks.Console" ],
"WriteTo": [ "Console" ],
"Filter": [
{
"Name": "ByIncludingOnly",
"Args": { "inclusionPredicate": "Matching.WithProperty<string>(\"Word\", w => w.StartsWith(\"a\"))" }
},
{
"Name": "ByExcluding",
"Args": { "exclusionPredicate": "Matching.WithProperty<string>(\"Word\", w => w.Equals(\"ad\"))" }
}
]
}
} If the Serilog config package were updated for destructuring, it would look like this: {
"Serilog": {
"Using": [ "Serilog.Sinks.Console" ],
"WriteTo": [ "Console" ],
"Destructure": [
{
"Name": "ByTransformingWhere",
"Args": {
"predicate": "t => typeof(Account).Equals(t)",
"transformedType": "Sample.Account",
"transformation": "a => new { a.id, a.Username, a.AccountType }"
}
}
]
}
} |
Thanks @MV10! I'm against bringing more functionality into this repository for the time being, just because we're already having a hard time getting optimal support for the features already here :-) .. having this via your repo as an add-on package would be great for those who want it in the interim. Thoughts? |
Honestly, to me it doesn't seem to have a lot of value as a stand-alone since that basically relegates it to a Filter extension. The destructure support is pointless without corresponding support in this Config package. Frankly, I'm just one guy, not an organization of any sort or anyone of any particular renown, so I'm not too interested in supporting a package under my own name. I'd be happy to sign on as the S.S.Config guy if that's what it takes, it's a little different when I'm not flying solo, and I'm pretty heavily invested in the future of the Config package. (Plus I enjoy it, for whatever that's worth.) Or we can chalk it up to a good time and close as won't implement. I'm pretty sure there is no other (reasonable) route to predicate/expression support from config. |
I suppose I could add support for the other less-complex destructure features (i.e. add But I guess that's more functionality in the package, too...? |
Starting work on a PR for the other less complicated destructure syntax (i.e. that won't require any new features) so that delegates can be added as a separate extensions package. So, @zippy1981 I guess the answer to your end-game question is that I'll be managing a nuget package with this feature. Give me a bit more time, ultimately I want parity with "real" Serilog package quality -- docs, tests, Travis, and AppVeyor support. Or if anyone else feels like working on a PR in my repo for that (while I add destructure to Config), please feel free. |
This is finally live and available on NuGet. |
I have migrated recently from defining Serilog config in code to appsettings.json.
I would like to be able to specify things like
.Destructure.ByTransforming<CultureInfo>(c => new { c.LCID, c.Name })
. Even simplified syntax would be fine.The text was updated successfully, but these errors were encountered: